Re: [PATCH 0/2] media: ov7670: fix regressions caused by "hook s_power onto v4l2 core"
On Tue, 2019-03-12 at 00:36 +0900, Akinobu Mita wrote: > This patchset fixes the problems introduced by recent change to ov7670. > > Akinobu Mita (2): > media: ov7670: restore default settings after power-up > media: ov7670: don't access registers when the device is powered off > > drivers/media/i2c/ov7670.c | 32 +++- > 1 file changed, 27 insertions(+), 5 deletions(-) > > Cc: Lubomir Rintel > Cc: Jonathan Corbet > Cc: Sakari Ailus > Cc: Mauro Carvalho Chehab For the both patches in the set: Reviewed-by: Lubomir Rintel Tested-by: Lubomir Rintel Thank you, Lubo
Re: [PATCH v4 0/5] ov7670 fixes
On Tue, 2019-01-15 at 12:40 +0200, Sakari Ailus wrote: > On Tue, Jan 15, 2019 at 09:54:43AM +0100, Lubomir Rintel wrote: > > Hi, > > > > here are the ov7670 patches originally from the "media: make Marvell camera > > work on DT-based OLPC XO-1.75" updated to apply cleanly on top of > > master as requested. > > > > I've also added "ov7670: Remove useless use of a ret variable" with my Ack > > slapped on it. > > Hi Lubomir, > > It seems the end result compiles but the intermedia patches do not. Could > you resend, please? I'll replace the patches in my tree with a new version > then... Seems like the order got messed up. It should be sufficient to reorder the patches like this: pick f42ae764598a ov7670: Remove useless use of a ret variable pick 5be2efe0e5bb media: ov7670: split register setting from set_fmt() logic pick 38eed963866e media: ov7670: split register setting from set_framerate() logic pick 97be3c31de46 media: ov7670: hook s_power onto v4l2 core pick 56c292d92642 media: ov7670: control clock along with power (With "git rebase -i 87680151c22eee5b3bb6361fb8d18d765a9d8aff") > > Thanks. Cheers, Lubo
[PATCH v4 1/5] ov7670: Remove useless use of a ret variable
From: Sakari Ailus Instead of assigning the return value to ret and then checking and returning it, just return the value to the caller directly. The success value is always 0. Signed-off-by: Sakari Ailus Acked-by: Lubomir Rintel --- drivers/media/i2c/ov7670.c | 6 +- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/drivers/media/i2c/ov7670.c b/drivers/media/i2c/ov7670.c index 4939a83b50e4..61c47c61c693 100644 --- a/drivers/media/i2c/ov7670.c +++ b/drivers/media/i2c/ov7670.c @@ -859,11 +859,7 @@ static int ov7675_set_framerate(struct v4l2_subdev *sd, /* Recalculate frame rate */ ov7675_get_framerate(sd, tpf); - ret = ov7670_write(sd, REG_CLKRC, info->clkrc); - if (ret < 0) - return ret; - - return 0; + return ov7670_write(sd, REG_CLKRC, info->clkrc); } static void ov7670_get_framerate_legacy(struct v4l2_subdev *sd, -- 2.20.1
[PATCH v4 4/5] media: ov7670: split register setting from set_fmt() logic
This will allow us to restore the last set format after the device returns
from a power off.
Signed-off-by: Lubomir Rintel
---
drivers/media/i2c/ov7670.c | 80 ++
1 file changed, 46 insertions(+), 34 deletions(-)
diff --git a/drivers/media/i2c/ov7670.c b/drivers/media/i2c/ov7670.c
index 93c055502bb9..d0f40d5f6ca0 100644
--- a/drivers/media/i2c/ov7670.c
+++ b/drivers/media/i2c/ov7670.c
@@ -241,6 +241,7 @@ struct ov7670_info {
};
struct v4l2_mbus_framefmt format;
struct ov7670_format_struct *fmt; /* Current format */
+ struct ov7670_win_size *wsize;
struct clk *clk;
int on;
struct gpio_desc *resetb_gpio;
@@ -1001,48 +1002,20 @@ static int ov7670_try_fmt_internal(struct v4l2_subdev
*sd,
return 0;
}
-/*
- * Set a format.
- */
-static int ov7670_set_fmt(struct v4l2_subdev *sd,
- struct v4l2_subdev_pad_config *cfg,
- struct v4l2_subdev_format *format)
+static int ov7670_apply_fmt(struct v4l2_subdev *sd)
{
- struct ov7670_format_struct *ovfmt;
- struct ov7670_win_size *wsize;
struct ov7670_info *info = to_state(sd);
-#ifdef CONFIG_VIDEO_V4L2_SUBDEV_API
- struct v4l2_mbus_framefmt *mbus_fmt;
-#endif
+ struct ov7670_win_size *wsize = info->wsize;
unsigned char com7, com10 = 0;
int ret;
- if (format->pad)
- return -EINVAL;
-
- if (format->which == V4L2_SUBDEV_FORMAT_TRY) {
- ret = ov7670_try_fmt_internal(sd, &format->format, NULL, NULL);
- if (ret)
- return ret;
-#ifdef CONFIG_VIDEO_V4L2_SUBDEV_API
- mbus_fmt = v4l2_subdev_get_try_format(sd, cfg, format->pad);
- *mbus_fmt = format->format;
- return 0;
-#else
- return -ENOTTY;
-#endif
- }
-
- ret = ov7670_try_fmt_internal(sd, &format->format, &ovfmt, &wsize);
- if (ret)
- return ret;
/*
* COM7 is a pain in the ass, it doesn't like to be read then
* quickly written afterward. But we have everything we need
* to set it absolutely here, as long as the format-specific
* register sets list it first.
*/
- com7 = ovfmt->regs[0].value;
+ com7 = info->fmt->regs[0].value;
com7 |= wsize->com7_bit;
ret = ov7670_write(sd, REG_COM7, com7);
if (ret)
@@ -1064,7 +1037,7 @@ static int ov7670_set_fmt(struct v4l2_subdev *sd,
/*
* Now write the rest of the array. Also store start/stops
*/
- ret = ov7670_write_array(sd, ovfmt->regs + 1);
+ ret = ov7670_write_array(sd, info->fmt->regs + 1);
if (ret)
return ret;
@@ -1079,8 +1052,6 @@ static int ov7670_set_fmt(struct v4l2_subdev *sd,
return ret;
}
- info->fmt = ovfmt;
-
/*
* If we're running RGB565, we must rewrite clkrc after setting
* the other parameters or the image looks poor. If we're *not*
@@ -1098,6 +1069,46 @@ static int ov7670_set_fmt(struct v4l2_subdev *sd,
return 0;
}
+/*
+ * Set a format.
+ */
+static int ov7670_set_fmt(struct v4l2_subdev *sd,
+ struct v4l2_subdev_pad_config *cfg,
+ struct v4l2_subdev_format *format)
+{
+ struct ov7670_info *info = to_state(sd);
+#ifdef CONFIG_VIDEO_V4L2_SUBDEV_API
+ struct v4l2_mbus_framefmt *mbus_fmt;
+#endif
+ int ret;
+
+ if (format->pad)
+ return -EINVAL;
+
+ if (format->which == V4L2_SUBDEV_FORMAT_TRY) {
+ ret = ov7670_try_fmt_internal(sd, &format->format, NULL, NULL);
+ if (ret)
+ return ret;
+#ifdef CONFIG_VIDEO_V4L2_SUBDEV_API
+ mbus_fmt = v4l2_subdev_get_try_format(sd, cfg, format->pad);
+ *mbus_fmt = format->format;
+ return 0;
+#else
+ return -ENOTTY;
+#endif
+ }
+
+ ret = ov7670_try_fmt_internal(sd, &format->format, &info->fmt,
&info->wsize);
+ if (ret)
+ return ret;
+
+ ret = ov7670_apply_fmt(sd);
+ if (ret)
+ return ret;
+
+ return 0;
+}
+
static int ov7670_get_fmt(struct v4l2_subdev *sd,
struct v4l2_subdev_pad_config *cfg,
struct v4l2_subdev_format *format)
@@ -1882,6 +1893,7 @@ static int ov7670_probe(struct i2c_client *client,
info->devtype = &ov7670_devdata[id->driver_data];
info->fmt = &ov7670_formats[0];
+ info->wsize = &info->devtype->win_sizes[0];
ov7670_get_default_format(sd, &info->format);
--
2.20.1
[PATCH v4 2/5] media: ov7670: hook s_power onto v4l2 core
The commit 71862f63f351 ("media: ov7670: Add the ov7670_s_power function")
added a power control routing. However, it was not good enough to use as
a s_power() callback: it merely flipped on the power GPIOs without
restoring the register settings.
Fix this now and register an actual power callback.
Signed-off-by: Lubomir Rintel
Signed-off-by: Sakari Ailus
---
drivers/media/i2c/ov7670.c | 50 +-
1 file changed, 44 insertions(+), 6 deletions(-)
diff --git a/drivers/media/i2c/ov7670.c b/drivers/media/i2c/ov7670.c
index 61c47c61c693..4679aa9dc430 100644
--- a/drivers/media/i2c/ov7670.c
+++ b/drivers/media/i2c/ov7670.c
@@ -242,6 +242,7 @@ struct ov7670_info {
struct v4l2_mbus_framefmt format;
struct ov7670_format_struct *fmt; /* Current format */
struct clk *clk;
+ int on;
struct gpio_desc *resetb_gpio;
struct gpio_desc *pwdn_gpio;
unsigned int mbus_config; /* Media bus configuration flags */
@@ -1603,19 +1604,54 @@ static int ov7670_s_register(struct v4l2_subdev *sd,
const struct v4l2_dbg_regis
}
#endif
-static int ov7670_s_power(struct v4l2_subdev *sd, int on)
+static void ov7670_power_on(struct v4l2_subdev *sd)
{
struct ov7670_info *info = to_state(sd);
+ if (info->on)
+ return;
+
if (info->pwdn_gpio)
- gpiod_set_value(info->pwdn_gpio, !on);
- if (on && info->resetb_gpio) {
+ gpiod_set_value(info->pwdn_gpio, 0);
+ if (info->resetb_gpio) {
gpiod_set_value(info->resetb_gpio, 1);
usleep_range(500, 1000);
gpiod_set_value(info->resetb_gpio, 0);
usleep_range(3000, 5000);
}
+ info->on = true;
+}
+
+static void ov7670_power_off(struct v4l2_subdev *sd)
+{
+ struct ov7670_info *info = to_state(sd);
+
+ if (!info->on)
+ return;
+
+ if (info->pwdn_gpio)
+ gpiod_set_value(info->pwdn_gpio, 1);
+
+ info->on = false;
+}
+
+static int ov7670_s_power(struct v4l2_subdev *sd, int on)
+{
+ struct ov7670_info *info = to_state(sd);
+
+ if (info->on == on)
+ return 0;
+
+ if (on) {
+ ov7670_power_on (sd);
+ ov7670_apply_fmt(sd);
+ ov7675_apply_framerate(sd);
+ v4l2_ctrl_handler_setup(&info->hdl);
+ } else {
+ ov7670_power_off (sd);
+ }
+
return 0;
}
@@ -1648,6 +1684,7 @@ static int ov7670_open(struct v4l2_subdev *sd, struct
v4l2_subdev_fh *fh)
static const struct v4l2_subdev_core_ops ov7670_core_ops = {
.reset = ov7670_reset,
.init = ov7670_init,
+ .s_power = ov7670_s_power,
.log_status = v4l2_ctrl_subdev_log_status,
.subscribe_event = v4l2_ctrl_subdev_subscribe_event,
.unsubscribe_event = v4l2_event_subdev_unsubscribe,
@@ -1812,6 +1849,7 @@ static int ov7670_probe(struct i2c_client *client,
else
return ret;
}
+
if (info->clk) {
ret = clk_prepare_enable(info->clk);
if (ret)
@@ -1828,7 +1866,7 @@ static int ov7670_probe(struct i2c_client *client,
if (ret)
goto clk_disable;
- ov7670_s_power(sd, 1);
+ ov7670_power_on(sd);
/* Make sure it's an ov7670 */
ret = ov7670_detect(sd);
@@ -1915,7 +1953,7 @@ static int ov7670_probe(struct i2c_client *client,
hdl_free:
v4l2_ctrl_handler_free(&info->hdl);
power_off:
- ov7670_s_power(sd, 0);
+ ov7670_power_off(sd);
clk_disable:
clk_disable_unprepare(info->clk);
return ret;
@@ -1931,7 +1969,7 @@ static int ov7670_remove(struct i2c_client *client)
v4l2_ctrl_handler_free(&info->hdl);
clk_disable_unprepare(info->clk);
media_entity_cleanup(&info->sd.entity);
- ov7670_s_power(sd, 0);
+ ov7670_power_off(sd);
return 0;
}
--
2.20.1
[PATCH v4 0/5] ov7670 fixes
Hi, here are the ov7670 patches originally from the "media: make Marvell camera work on DT-based OLPC XO-1.75" updated to apply cleanly on top of master as requested. I've also added "ov7670: Remove useless use of a ret variable" with my Ack slapped on it. Lubo
[PATCH v4 5/5] media: ov7670: split register setting from set_framerate() logic
This will allow us to restore the last set frame rate after the device
returns from a power off.
Signed-off-by: Lubomir Rintel
---
drivers/media/i2c/ov7670.c | 26 ++
1 file changed, 14 insertions(+), 12 deletions(-)
diff --git a/drivers/media/i2c/ov7670.c b/drivers/media/i2c/ov7670.c
index d0f40d5f6ca0..6f9a53d4dcfc 100644
--- a/drivers/media/i2c/ov7670.c
+++ b/drivers/media/i2c/ov7670.c
@@ -812,13 +812,24 @@ static void ov7675_get_framerate(struct v4l2_subdev *sd,
(4 * clkrc);
}
+static int ov7675_apply_framerate(struct v4l2_subdev *sd)
+{
+ struct ov7670_info *info = to_state(sd);
+ int ret;
+
+ ret = ov7670_write(sd, REG_CLKRC, info->clkrc);
+ if (ret < 0)
+ return ret;
+
+ return ov7670_write(sd, REG_DBLV, info->pll_bypass ? DBLV_BYPASS :
DBLV_X4);
+}
+
static int ov7675_set_framerate(struct v4l2_subdev *sd,
struct v4l2_fract *tpf)
{
struct ov7670_info *info = to_state(sd);
u32 clkrc;
int pll_factor;
- int ret;
/*
* The formula is fps = 5/4*pixclk for YUV/RGB and
@@ -827,19 +838,10 @@ static int ov7675_set_framerate(struct v4l2_subdev *sd,
* pixclk = clock_speed / (clkrc + 1) * PLLfactor
*
*/
- if (info->pll_bypass) {
- pll_factor = 1;
- ret = ov7670_write(sd, REG_DBLV, DBLV_BYPASS);
- } else {
- pll_factor = PLL_FACTOR;
- ret = ov7670_write(sd, REG_DBLV, DBLV_X4);
- }
- if (ret < 0)
- return ret;
-
if (tpf->numerator == 0 || tpf->denominator == 0) {
clkrc = 0;
} else {
+ pll_factor = info->pll_bypass ? 1 : PLL_FACTOR;
clkrc = (5 * pll_factor * info->clock_speed * tpf->numerator) /
(4 * tpf->denominator);
if (info->fmt->mbus_code == MEDIA_BUS_FMT_SBGGR8_1X8)
@@ -861,7 +863,7 @@ static int ov7675_set_framerate(struct v4l2_subdev *sd,
/* Recalculate frame rate */
ov7675_get_framerate(sd, tpf);
- return ov7670_write(sd, REG_CLKRC, info->clkrc);
+ return ov7675_apply_framerate(sd);
}
static void ov7670_get_framerate_legacy(struct v4l2_subdev *sd,
--
2.20.1
[PATCH v4 3/5] media: ov7670: control clock along with power
This provides more power saving when the sensor is off.
While at that, do the delay on power/clock enable even if the sensor driver
itself doesn't control the GPIOs. This is required for the OLPC XO-1
platform, that lacks the proper power/reset properties in its DT, but
needs the delay after the sensor is clocked up.
Signed-off-by: Lubomir Rintel
Signed-off-by: Sakari Ailus
---
drivers/media/i2c/ov7670.c | 30 ++
1 file changed, 14 insertions(+), 16 deletions(-)
diff --git a/drivers/media/i2c/ov7670.c b/drivers/media/i2c/ov7670.c
index 4679aa9dc430..93c055502bb9 100644
--- a/drivers/media/i2c/ov7670.c
+++ b/drivers/media/i2c/ov7670.c
@@ -1611,14 +1611,17 @@ static void ov7670_power_on(struct v4l2_subdev *sd)
if (info->on)
return;
+ clk_prepare_enable(info->clk);
+
if (info->pwdn_gpio)
gpiod_set_value(info->pwdn_gpio, 0);
if (info->resetb_gpio) {
gpiod_set_value(info->resetb_gpio, 1);
usleep_range(500, 1000);
gpiod_set_value(info->resetb_gpio, 0);
- usleep_range(3000, 5000);
}
+ if (info->pwdn_gpio || info->resetb_gpio || info->clk)
+ usleep_range(3000, 5000);
info->on = true;
}
@@ -1630,6 +1633,8 @@ static void ov7670_power_off(struct v4l2_subdev *sd)
if (!info->on)
return;
+ clk_disable_unprepare(info->clk);
+
if (info->pwdn_gpio)
gpiod_set_value(info->pwdn_gpio, 1);
@@ -1850,24 +1855,20 @@ static int ov7670_probe(struct i2c_client *client,
return ret;
}
- if (info->clk) {
- ret = clk_prepare_enable(info->clk);
- if (ret)
- return ret;
+ ret = ov7670_init_gpio(client, info);
+ if (ret)
+ return ret;
+ ov7670_power_on(sd);
+
+ if (info->clk) {
info->clock_speed = clk_get_rate(info->clk) / 100;
if (info->clock_speed < 10 || info->clock_speed > 48) {
ret = -EINVAL;
- goto clk_disable;
+ goto power_off;
}
}
- ret = ov7670_init_gpio(client, info);
- if (ret)
- goto clk_disable;
-
- ov7670_power_on(sd);
-
/* Make sure it's an ov7670 */
ret = ov7670_detect(sd);
if (ret) {
@@ -1946,6 +1947,7 @@ static int ov7670_probe(struct i2c_client *client,
if (ret < 0)
goto entity_cleanup;
+ ov7670_power_off(sd);
return 0;
entity_cleanup:
@@ -1954,12 +1956,9 @@ static int ov7670_probe(struct i2c_client *client,
v4l2_ctrl_handler_free(&info->hdl);
power_off:
ov7670_power_off(sd);
-clk_disable:
- clk_disable_unprepare(info->clk);
return ret;
}
-
static int ov7670_remove(struct i2c_client *client)
{
struct v4l2_subdev *sd = i2c_get_clientdata(client);
@@ -1967,7 +1966,6 @@ static int ov7670_remove(struct i2c_client *client)
v4l2_async_unregister_subdev(sd);
v4l2_ctrl_handler_free(&info->hdl);
- clk_disable_unprepare(info->clk);
media_entity_cleanup(&info->sd.entity);
ov7670_power_off(sd);
return 0;
--
2.20.1
Re: [PATCH v3 02/14] media: ov7670: split register setting from set_framerate() logic
On Tue, 2019-01-15 at 01:03 +0200, Sakari Ailus wrote:
> On Tue, Nov 20, 2018 at 11:03:07AM +0100, Lubomir Rintel wrote:
> > This will allow us to restore the last set frame rate after the device
> > returns from a power off.
> >
> > Signed-off-by: Lubomir Rintel
> >
> > ---
> > Changes since v2:
> > - This patch was added to the series
> >
> > drivers/media/i2c/ov7670.c | 30 ++
> > 1 file changed, 14 insertions(+), 16 deletions(-)
> >
> > diff --git a/drivers/media/i2c/ov7670.c b/drivers/media/i2c/ov7670.c
> > index ee2302fbdeee..ead0c360df33 100644
> > --- a/drivers/media/i2c/ov7670.c
> > +++ b/drivers/media/i2c/ov7670.c
> > @@ -810,13 +810,24 @@ static void ov7675_get_framerate(struct v4l2_subdev
> > *sd,
> > (4 * clkrc);
> > }
> >
> > +static int ov7675_apply_framerate(struct v4l2_subdev *sd)
> > +{
> > + struct ov7670_info *info = to_state(sd);
> > + int ret;
> > +
> > + ret = ov7670_write(sd, REG_CLKRC, info->clkrc);
> > + if (ret < 0)
> > + return ret;
> > +
> > + return ov7670_write(sd, REG_DBLV, info->pll_bypass ? DBLV_BYPASS :
> > DBLV_X4);
> > +}
> > +
> > static int ov7675_set_framerate(struct v4l2_subdev *sd,
> > struct v4l2_fract *tpf)
> > {
> > struct ov7670_info *info = to_state(sd);
> > u32 clkrc;
> > int pll_factor;
> > - int ret;
> >
> > /*
> > * The formula is fps = 5/4*pixclk for YUV/RGB and
> > @@ -825,19 +836,10 @@ static int ov7675_set_framerate(struct v4l2_subdev
> > *sd,
> > * pixclk = clock_speed / (clkrc + 1) * PLLfactor
> > *
> > */
> > - if (info->pll_bypass) {
> > - pll_factor = 1;
> > - ret = ov7670_write(sd, REG_DBLV, DBLV_BYPASS);
> > - } else {
> > - pll_factor = PLL_FACTOR;
> > - ret = ov7670_write(sd, REG_DBLV, DBLV_X4);
> > - }
> > - if (ret < 0)
> > - return ret;
> > -
> > if (tpf->numerator == 0 || tpf->denominator == 0) {
> > clkrc = 0;
> > } else {
> > + pll_factor = info->pll_bypass ? 1 : PLL_FACTOR;
> > clkrc = (5 * pll_factor * info->clock_speed * tpf->numerator) /
> > (4 * tpf->denominator);
> > if (info->fmt->mbus_code == MEDIA_BUS_FMT_SBGGR8_1X8)
> > @@ -859,11 +861,7 @@ static int ov7675_set_framerate(struct v4l2_subdev *sd,
> > /* Recalculate frame rate */
> > ov7675_get_framerate(sd, tpf);
> >
> > - ret = ov7670_write(sd, REG_CLKRC, info->clkrc);
> > - if (ret < 0)
> > - return ret;
> > -
> > - return ov7670_write(sd, REG_DBLV, DBLV_X4);
> > + return ov7675_apply_framerate(sd);
> > }
> >
> > static void ov7670_get_framerate_legacy(struct v4l2_subdev *sd,
>
> Unfortunately this one no longer applies due to Jacopo's patch "v4l2: i2c:
> ov7670: Fix PLL bypass register values" in my tree. Could you resend it if
> you still think it's useful, please?
>
> My tree is here:
>
> https://git.linuxtv.org/sailus/media_tree.git/log/>
>
> I've applied the rest there, with a trivial patch I'll send briefly.
Thank you.
It seems that you've accidentally attached the hunks of that patch to a
previous one though ("media: ov7670: split register setting from
set_fmt() logic"). There's just one conflicted line to resolve left.
I have a branch based on your tree that reverts the patch first and
then applies the two cleanly:
git pull https://github.com/hackerspace/olpc-xo175-linux.git lr/ov7670
Feel free to pull, or, if you're fine with rebasing your tree, remove
the patch from your tree and then just pick the two.
Alternatively, if you prefer a patch by e-mail, please just let me know
when you've fixed things up in your tree.
Cheers
Lubo
>
> Thanks.
>
Re: [PATCH v3 03/14] media: ov7670: hook s_power onto v4l2 core
On Thu, 2019-01-10 at 18:59 +0200, Sakari Ailus wrote:
> Hi Lubomir,
>
> On Wed, Nov 28, 2018 at 12:29:33PM +0100, Lubomir Rintel wrote:
> > On Thu, 2018-11-22 at 14:21 +0200, Sakari Ailus wrote:
> > > Hi Lubomir,
> > >
> > > On Tue, Nov 20, 2018 at 11:03:08AM +0100, Lubomir Rintel wrote:
> > > > The commit 71862f63f351 ("media: ov7670: Add the ov7670_s_power
> > > > function")
> > > > added a power control routing. However, it was not good enough to use as
> > > > a s_power() callback: it merely flipped on the power GPIOs without
> > > > restoring the register settings.
> > > >
> > > > Fix this now and register an actual power callback.
> > > >
> > > > Signed-off-by: Lubomir Rintel
> > > >
> > > > ---
> > > > Changes since v2:
> > > > - Restore the controls, format and frame rate on power on
> > > >
> > > > drivers/media/i2c/ov7670.c | 50 +-
> > > > 1 file changed, 44 insertions(+), 6 deletions(-)
> > > >
> > > > diff --git a/drivers/media/i2c/ov7670.c b/drivers/media/i2c/ov7670.c
> > > > index ead0c360df33..cbaab60c 100644
> > > > --- a/drivers/media/i2c/ov7670.c
> > > > +++ b/drivers/media/i2c/ov7670.c
> ...
> > > > @@ -1945,7 +1983,7 @@ static int ov7670_remove(struct i2c_client
> > > > *client)
> > > > v4l2_ctrl_handler_free(&info->hdl);
> > > > clk_disable_unprepare(info->clk);
> > > > media_entity_cleanup(&info->sd.entity);
> > > > - ov7670_s_power(sd, 0);
> > > > + ov7670_power_off(sd);
> > > > return 0;
> > > > }
> > > >
> > >
> > > Could you consider instead switching to runtime PM? A few drivers such as
> > > the ov2685 driver does that already.
> >
> > Yes, I'll take a look. Thanks for the suggestion. I didn't know such
> > thing exists, so it may take some time for me to grasp it though.
>
> I'm be tempted to merge the ov7670 patches as they significantly improve
> the driver, even if we're at the moment missing the runtime PM conversion.
> It could be done later on as well.
>
> Would that be ok for you?
Yes, I'm fine with it. In fact, I'm happy about it.
I'd eventually like to implement the runtime PM, but it's not going to
happen before 5.1 merge window.
Cheers
Lubo
Re: [PATCH V2] mm: Replace all open encodings for NUMA_NO_NODE
On Wed, 2018-12-05 at 17:01 +0530, Anshuman Khandual wrote: > > On 12/05/2018 02:56 AM, Lubomir Rintel wrote: > > On Mon, 2018-11-26 at 17:56 +0530, Anshuman Khandual wrote: > > > At present there are multiple places where invalid node number is encoded > > > as -1. Even though implicitly understood it is always better to have > > > macros > > > in there. Replace these open encodings for an invalid node number with the > > > global macro NUMA_NO_NODE. This helps remove NUMA related assumptions like > > > 'invalid node' from various places redirecting them to a common > > > definition. > > > > > > Signed-off-by: Anshuman Khandual > > > --- > > > Changes in V2: > > > > > > - Added inclusion of 'numa.h' header at various places per Andrew > > > - Updated 'dev_to_node' to use NUMA_NO_NODE instead per Vinod > > > > > > Changes in V1: (https://lkml.org/lkml/2018/11/23/485) > > > > > > - Dropped OCFS2 changes per Joseph > > > - Dropped media/video drivers changes per Hans > > > > > > RFC - https://patchwork.kernel.org/patch/10678035/ > > > > > > Build tested this with multiple cross compiler options like alpha, sparc, > > > arm64, x86, powerpc, powerpc64le etc with their default config which might > > > not have compiled tested all driver related changes. I will appreciate > > > folks giving this a test in their respective build environment. > > > > > > All these places for replacement were found by running the following grep > > > patterns on the entire kernel code. Please let me know if this might have > > > missed some instances. This might also have replaced some false positives. > > > I will appreciate suggestions, inputs and review. > > > > > > 1. git grep "nid == -1" > > > 2. git grep "node == -1" > > > 3. git grep "nid = -1" > > > 4. git grep "node = -1" > > > > > > arch/alpha/include/asm/topology.h | 3 ++- > > > arch/ia64/kernel/numa.c | 2 +- > > > arch/ia64/mm/discontig.c | 6 +++--- > > > arch/ia64/sn/kernel/io_common.c | 3 ++- > > > arch/powerpc/include/asm/pci-bridge.h | 3 ++- > > > arch/powerpc/kernel/paca.c| 3 ++- > > > arch/powerpc/kernel/pci-common.c | 3 ++- > > > arch/powerpc/mm/numa.c| 14 +++--- > > > arch/powerpc/platforms/powernv/memtrace.c | 5 +++-- > > > arch/sparc/kernel/auxio_32.c | 3 ++- > > > arch/sparc/kernel/pci_fire.c | 3 ++- > > > arch/sparc/kernel/pci_schizo.c| 3 ++- > > > arch/sparc/kernel/pcic.c | 7 --- > > > arch/sparc/kernel/psycho_common.c | 3 ++- > > > arch/sparc/kernel/sbus.c | 3 ++- > > > arch/sparc/mm/init_64.c | 6 +++--- > > > arch/sparc/prom/init_32.c | 3 ++- > > > arch/sparc/prom/init_64.c | 5 +++-- > > > arch/sparc/prom/tree_32.c | 13 +++-- > > > arch/sparc/prom/tree_64.c | 19 ++- > > > arch/x86/include/asm/pci.h| 3 ++- > > > arch/x86/kernel/apic/x2apic_uv_x.c| 7 --- > > > arch/x86/kernel/smpboot.c | 3 ++- > > > arch/x86/platform/olpc/olpc_dt.c | 17 + > > > drivers/block/mtip32xx/mtip32xx.c | 5 +++-- > > > drivers/dma/dmaengine.c | 4 +++- > > > drivers/infiniband/hw/hfi1/affinity.c | 3 ++- > > > drivers/infiniband/hw/hfi1/init.c | 3 ++- > > > drivers/iommu/dmar.c | 5 +++-- > > > drivers/iommu/intel-iommu.c | 3 ++- > > > drivers/misc/sgi-xp/xpc_uv.c | 3 ++- > > > drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 5 +++-- > > > include/linux/device.h| 2 +- > > > init/init_task.c | 3 ++- > > > kernel/kthread.c | 3 ++- > > > kernel/sched/fair.c | 15 --- > > > lib/cpumask.c | 3 ++- > > > mm/huge_memory.c |
Re: [PATCH V2] mm: Replace all open encodings for NUMA_NO_NODE
On Mon, 2018-11-26 at 17:56 +0530, Anshuman Khandual wrote: > At present there are multiple places where invalid node number is encoded > as -1. Even though implicitly understood it is always better to have macros > in there. Replace these open encodings for an invalid node number with the > global macro NUMA_NO_NODE. This helps remove NUMA related assumptions like > 'invalid node' from various places redirecting them to a common definition. > > Signed-off-by: Anshuman Khandual > --- > Changes in V2: > > - Added inclusion of 'numa.h' header at various places per Andrew > - Updated 'dev_to_node' to use NUMA_NO_NODE instead per Vinod > > Changes in V1: (https://lkml.org/lkml/2018/11/23/485) > > - Dropped OCFS2 changes per Joseph > - Dropped media/video drivers changes per Hans > > RFC - https://patchwork.kernel.org/patch/10678035/ > > Build tested this with multiple cross compiler options like alpha, sparc, > arm64, x86, powerpc, powerpc64le etc with their default config which might > not have compiled tested all driver related changes. I will appreciate > folks giving this a test in their respective build environment. > > All these places for replacement were found by running the following grep > patterns on the entire kernel code. Please let me know if this might have > missed some instances. This might also have replaced some false positives. > I will appreciate suggestions, inputs and review. > > 1. git grep "nid == -1" > 2. git grep "node == -1" > 3. git grep "nid = -1" > 4. git grep "node = -1" > > arch/alpha/include/asm/topology.h | 3 ++- > arch/ia64/kernel/numa.c | 2 +- > arch/ia64/mm/discontig.c | 6 +++--- > arch/ia64/sn/kernel/io_common.c | 3 ++- > arch/powerpc/include/asm/pci-bridge.h | 3 ++- > arch/powerpc/kernel/paca.c| 3 ++- > arch/powerpc/kernel/pci-common.c | 3 ++- > arch/powerpc/mm/numa.c| 14 +++--- > arch/powerpc/platforms/powernv/memtrace.c | 5 +++-- > arch/sparc/kernel/auxio_32.c | 3 ++- > arch/sparc/kernel/pci_fire.c | 3 ++- > arch/sparc/kernel/pci_schizo.c| 3 ++- > arch/sparc/kernel/pcic.c | 7 --- > arch/sparc/kernel/psycho_common.c | 3 ++- > arch/sparc/kernel/sbus.c | 3 ++- > arch/sparc/mm/init_64.c | 6 +++--- > arch/sparc/prom/init_32.c | 3 ++- > arch/sparc/prom/init_64.c | 5 +++-- > arch/sparc/prom/tree_32.c | 13 +++-- > arch/sparc/prom/tree_64.c | 19 ++- > arch/x86/include/asm/pci.h| 3 ++- > arch/x86/kernel/apic/x2apic_uv_x.c| 7 --- > arch/x86/kernel/smpboot.c | 3 ++- > arch/x86/platform/olpc/olpc_dt.c | 17 + > drivers/block/mtip32xx/mtip32xx.c | 5 +++-- > drivers/dma/dmaengine.c | 4 +++- > drivers/infiniband/hw/hfi1/affinity.c | 3 ++- > drivers/infiniband/hw/hfi1/init.c | 3 ++- > drivers/iommu/dmar.c | 5 +++-- > drivers/iommu/intel-iommu.c | 3 ++- > drivers/misc/sgi-xp/xpc_uv.c | 3 ++- > drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 5 +++-- > include/linux/device.h| 2 +- > init/init_task.c | 3 ++- > kernel/kthread.c | 3 ++- > kernel/sched/fair.c | 15 --- > lib/cpumask.c | 3 ++- > mm/huge_memory.c | 13 +++-- > mm/hugetlb.c | 3 ++- > mm/ksm.c | 2 +- > mm/memory.c | 7 --- > mm/memory_hotplug.c | 12 ++-- > mm/mempolicy.c| 2 +- > mm/page_alloc.c | 4 ++-- > mm/page_ext.c | 2 +- > net/core/pktgen.c | 3 ++- > net/qrtr/qrtr.c | 3 ++- > tools/perf/bench/numa.c | 6 +++--- > 48 files changed, 146 insertions(+), 108 deletions(-) Thanks for the patch. It seems to me that you've got a fairly large amount of it wrong though -- perhaps relying just on "git grep" alone is not the best idea. The diffstat is not all that big, it is entirely plausible to just review each hunk manually: just do a "git show -U20" to get some context. You get a NAK from me for the OLPC DT part, but I think at least the sparc/prom part also deals with device tree nodes and not NUMA nodes. More inline. Lubo > > diff --git a/arch/alpha/include/asm/topolog
Re: [PATCH 1/6] media: v4l2-subdev: stub v4l2_subdev_get_try_format()
On Mon, 2018-12-03 at 14:48 +0100, jacopo mondi wrote:
> Hi Lubomir,
>
> thanks for the patches
>
> On Wed, Nov 28, 2018 at 06:19:13PM +0100, Lubomir Rintel wrote:
> > Provide a dummy implementation when configured without
> > CONFIG_VIDEO_V4L2_SUBDEV_API to avoid ifdef dance in the drivers
> > that can
> > be built either with or without the option.
> >
> > Suggested-by: Jacopo Mondi
> > Signed-off-by: Lubomir Rintel
> > ---
> > include/media/v4l2-subdev.h | 11 +++
> > 1 file changed, 11 insertions(+)
> >
> > diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-
> > subdev.h
> > index 9102d6ca566e..906e28011bb4 100644
> > --- a/include/media/v4l2-subdev.h
> > +++ b/include/media/v4l2-subdev.h
> > @@ -967,6 +967,17 @@ static inline struct v4l2_rect
> > pad = 0;
> > return &cfg[pad].try_compose;
> > }
> > +
> > +#else /* !defined(CONFIG_VIDEO_V4L2_SUBDEV_API) */
> > +
> > +static inline struct v4l2_mbus_framefmt
> > +*v4l2_subdev_get_try_format(struct v4l2_subdev *sd,
> > + struct v4l2_subdev_pad_config *cfg,
> > + unsigned int pad)
> > +{
> > + return ERR_PTR(-ENOTTY);
> > +}
> > +
> > #endif
>
> While at there, what about doing the same for get_try_crop and
> get_try_compose? At lease provide stubs, I let you figure out if
> you're willing to fix callers too, it seems there are quite a few of
> them though
>
> $ git grep v4l2_subdev_get_try* drivers/media/ | grep -v '_format' |
> wc -l
> 44
I'd be happy to do that. However, the drivers that use those seem to
depend on CONFIG_VIDEO_V4L2_SUBDEV_API anyway. Should those
dependencies be eventually done away with?
Please pardon my ignorance -- I don't actually understand why would
anyone disable CONFIG_VIDEO_V4L2_SUBDEV_API.
I'll be following up with a v2 after I get a response from you. It will
address locking issues found with smatch: one introduced by my patch
and one that was there before.
Cheers,
Lubo
> > extern const struct v4l2_file_operations v4l2_subdev_fops;
> > --
> > 2.19.1
> >
[PATCH] [media] marvell-ccic: trivial fix to the datasheet URL
Signed-off-by: Lubomir Rintel --- drivers/media/platform/marvell-ccic/cafe-driver.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/media/platform/marvell-ccic/cafe-driver.c b/drivers/media/platform/marvell-ccic/cafe-driver.c index 2986cb4b88d0..8d00d9d8adff 100644 --- a/drivers/media/platform/marvell-ccic/cafe-driver.c +++ b/drivers/media/platform/marvell-ccic/cafe-driver.c @@ -4,7 +4,7 @@ * sensor. * * The data sheet for this device can be found at: - *http://www.marvell.com/products/pc_connectivity/88alp01/ + *http://wiki.laptop.org/images/5/5c/88ALP01_Datasheet_July_2007.pdf * * Copyright 2006-11 One Laptop Per Child Association, Inc. * Copyright 2006-11 Jonathan Corbet -- 2.19.1
[PATCH 1/6] media: v4l2-subdev: stub v4l2_subdev_get_try_format()
Provide a dummy implementation when configured without
CONFIG_VIDEO_V4L2_SUBDEV_API to avoid ifdef dance in the drivers that can
be built either with or without the option.
Suggested-by: Jacopo Mondi
Signed-off-by: Lubomir Rintel
---
include/media/v4l2-subdev.h | 11 +++
1 file changed, 11 insertions(+)
diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h
index 9102d6ca566e..906e28011bb4 100644
--- a/include/media/v4l2-subdev.h
+++ b/include/media/v4l2-subdev.h
@@ -967,6 +967,17 @@ static inline struct v4l2_rect
pad = 0;
return &cfg[pad].try_compose;
}
+
+#else /* !defined(CONFIG_VIDEO_V4L2_SUBDEV_API) */
+
+static inline struct v4l2_mbus_framefmt
+*v4l2_subdev_get_try_format(struct v4l2_subdev *sd,
+ struct v4l2_subdev_pad_config *cfg,
+ unsigned int pad)
+{
+ return ERR_PTR(-ENOTTY);
+}
+
#endif
extern const struct v4l2_file_operations v4l2_subdev_fops;
--
2.19.1
[PATCH 2/6] media: ov7740: get rid of extra ifdefs
Stubbed v4l2_subdev_get_try_format() will return a correct error when
configured without CONFIG_VIDEO_V4L2_SUBDEV_API.
Signed-off-by: Lubomir Rintel
---
drivers/media/i2c/ov7740.c | 25 ++---
1 file changed, 10 insertions(+), 15 deletions(-)
diff --git a/drivers/media/i2c/ov7740.c b/drivers/media/i2c/ov7740.c
index 6e9c233cfbe3..781ddcc743d4 100644
--- a/drivers/media/i2c/ov7740.c
+++ b/drivers/media/i2c/ov7740.c
@@ -780,9 +780,7 @@ static int ov7740_set_fmt(struct v4l2_subdev *sd,
struct ov7740 *ov7740 = container_of(sd, struct ov7740, subdev);
const struct ov7740_pixfmt *ovfmt;
const struct ov7740_framesize *fsize;
-#ifdef CONFIG_VIDEO_V4L2_SUBDEV_API
struct v4l2_mbus_framefmt *mbus_fmt;
-#endif
int ret;
mutex_lock(&ov7740->mutex);
@@ -795,16 +793,15 @@ static int ov7740_set_fmt(struct v4l2_subdev *sd,
ret = ov7740_try_fmt_internal(sd, &format->format, NULL, NULL);
if (ret)
goto error;
-#ifdef CONFIG_VIDEO_V4L2_SUBDEV_API
mbus_fmt = v4l2_subdev_get_try_format(sd, cfg, format->pad);
+ if (IS_ERR(mbus_fmt)) {
+ ret = PTR_ERR(mbus_fmt);
+ goto error;
+ }
*mbus_fmt = format->format;
mutex_unlock(&ov7740->mutex);
return 0;
-#else
- ret = -ENOTTY;
- goto error;
-#endif
}
ret = ov7740_try_fmt_internal(sd, &format->format, &ovfmt, &fsize);
@@ -827,20 +824,18 @@ static int ov7740_get_fmt(struct v4l2_subdev *sd,
struct v4l2_subdev_format *format)
{
struct ov7740 *ov7740 = container_of(sd, struct ov7740, subdev);
-#ifdef CONFIG_VIDEO_V4L2_SUBDEV_API
struct v4l2_mbus_framefmt *mbus_fmt;
-#endif
int ret = 0;
mutex_lock(&ov7740->mutex);
if (format->which == V4L2_SUBDEV_FORMAT_TRY) {
-#ifdef CONFIG_VIDEO_V4L2_SUBDEV_API
mbus_fmt = v4l2_subdev_get_try_format(sd, cfg, 0);
- format->format = *mbus_fmt;
- ret = 0;
-#else
- ret = -ENOTTY;
-#endif
+ if (IS_ERR(mbus_fmt)) {
+ ret = PTR_ERR(mbus_fmt);
+ } else {
+ format->format = *mbus_fmt;
+ ret = 0;
+ }
} else {
format->format = ov7740->format;
}
--
2.19.1
[PATCH 4/6] media: ov2680: get rid of extra ifdefs
Stubbed v4l2_subdev_get_try_format() will return a correct error when
configured without CONFIG_VIDEO_V4L2_SUBDEV_API.
Signed-off-by: Lubomir Rintel
---
drivers/media/i2c/ov2680.c | 17 ++---
1 file changed, 6 insertions(+), 11 deletions(-)
diff --git a/drivers/media/i2c/ov2680.c b/drivers/media/i2c/ov2680.c
index 0e34e15b67b3..f8b873aaacec 100644
--- a/drivers/media/i2c/ov2680.c
+++ b/drivers/media/i2c/ov2680.c
@@ -680,11 +680,9 @@ static int ov2680_get_fmt(struct v4l2_subdev *sd,
mutex_lock(&sensor->lock);
if (format->which == V4L2_SUBDEV_FORMAT_TRY) {
-#ifdef CONFIG_VIDEO_V4L2_SUBDEV_API
fmt = v4l2_subdev_get_try_format(&sensor->sd, cfg, format->pad);
-#else
- ret = -ENOTTY;
-#endif
+ if (IS_ERR(fmt))
+ return PTR_ERR(fmt);
} else {
fmt = &sensor->fmt;
}
@@ -703,9 +701,7 @@ static int ov2680_set_fmt(struct v4l2_subdev *sd,
{
struct ov2680_dev *sensor = to_ov2680_dev(sd);
struct v4l2_mbus_framefmt *fmt = &format->format;
-#ifdef CONFIG_VIDEO_V4L2_SUBDEV_API
struct v4l2_mbus_framefmt *try_fmt;
-#endif
const struct ov2680_mode_info *mode;
int ret = 0;
@@ -728,12 +724,11 @@ static int ov2680_set_fmt(struct v4l2_subdev *sd,
}
if (format->which == V4L2_SUBDEV_FORMAT_TRY) {
-#ifdef CONFIG_VIDEO_V4L2_SUBDEV_API
try_fmt = v4l2_subdev_get_try_format(sd, cfg, 0);
- format->format = *try_fmt;
-#else
- ret = -ENOTTY;
-#endif
+ if (IS_ERR(try_fmt))
+ ret = PTR_ERR(try_fmt);
+ else
+ format->format = *try_fmt;
goto unlock;
}
--
2.19.1
[PATCH 3/6] media: ov2659: get rid of extra ifdefs
Stubbed v4l2_subdev_get_try_format() will return a correct error when
configured without CONFIG_VIDEO_V4L2_SUBDEV_API.
Signed-off-by: Lubomir Rintel
---
drivers/media/i2c/ov2659.c | 12
1 file changed, 4 insertions(+), 8 deletions(-)
diff --git a/drivers/media/i2c/ov2659.c b/drivers/media/i2c/ov2659.c
index 799acce803fe..a66c12c8f278 100644
--- a/drivers/media/i2c/ov2659.c
+++ b/drivers/media/i2c/ov2659.c
@@ -1046,17 +1046,15 @@ static int ov2659_get_fmt(struct v4l2_subdev *sd,
dev_dbg(&client->dev, "ov2659_get_fmt\n");
if (fmt->which == V4L2_SUBDEV_FORMAT_TRY) {
-#ifdef CONFIG_VIDEO_V4L2_SUBDEV_API
struct v4l2_mbus_framefmt *mf;
mf = v4l2_subdev_get_try_format(sd, cfg, 0);
+ if (IS_ERR(mf))
+ return PTR_ERR(mf);
mutex_lock(&ov2659->lock);
fmt->format = *mf;
mutex_unlock(&ov2659->lock);
return 0;
-#else
- return -ENOTTY;
-#endif
}
mutex_lock(&ov2659->lock);
@@ -1126,12 +1124,10 @@ static int ov2659_set_fmt(struct v4l2_subdev *sd,
mutex_lock(&ov2659->lock);
if (fmt->which == V4L2_SUBDEV_FORMAT_TRY) {
-#ifdef CONFIG_VIDEO_V4L2_SUBDEV_API
mf = v4l2_subdev_get_try_format(sd, cfg, fmt->pad);
+ if (IS_ERR(mf))
+ return PTR_ERR(mf);
*mf = fmt->format;
-#else
- return -ENOTTY;
-#endif
} else {
s64 val;
--
2.19.1
[PATCH 6/6] media: ov7670: get rid of extra ifdefs
Stubbed v4l2_subdev_get_try_format() will return a correct error when
configured without CONFIG_VIDEO_V4L2_SUBDEV_API.
Signed-off-by: Lubomir Rintel
---
drivers/media/i2c/ov7670.c | 16
1 file changed, 4 insertions(+), 12 deletions(-)
diff --git a/drivers/media/i2c/ov7670.c b/drivers/media/i2c/ov7670.c
index bc68a3a5b4ec..7d1fdf51bd91 100644
--- a/drivers/media/i2c/ov7670.c
+++ b/drivers/media/i2c/ov7670.c
@@ -1013,9 +1013,7 @@ static int ov7670_set_fmt(struct v4l2_subdev *sd,
struct ov7670_format_struct *ovfmt;
struct ov7670_win_size *wsize;
struct ov7670_info *info = to_state(sd);
-#ifdef CONFIG_VIDEO_V4L2_SUBDEV_API
struct v4l2_mbus_framefmt *mbus_fmt;
-#endif
unsigned char com7, com10 = 0;
int ret;
@@ -1026,13 +1024,11 @@ static int ov7670_set_fmt(struct v4l2_subdev *sd,
ret = ov7670_try_fmt_internal(sd, &format->format, NULL, NULL);
if (ret)
return ret;
-#ifdef CONFIG_VIDEO_V4L2_SUBDEV_API
mbus_fmt = v4l2_subdev_get_try_format(sd, cfg, format->pad);
+ if (IS_ERR(mbus_fmt))
+ return PTR_ERR(mbus_fmt);
*mbus_fmt = format->format;
return 0;
-#else
- return -ENOTTY;
-#endif
}
ret = ov7670_try_fmt_internal(sd, &format->format, &ovfmt, &wsize);
@@ -1105,18 +1101,14 @@ static int ov7670_get_fmt(struct v4l2_subdev *sd,
struct v4l2_subdev_format *format)
{
struct ov7670_info *info = to_state(sd);
-#ifdef CONFIG_VIDEO_V4L2_SUBDEV_API
struct v4l2_mbus_framefmt *mbus_fmt;
-#endif
if (format->which == V4L2_SUBDEV_FORMAT_TRY) {
-#ifdef CONFIG_VIDEO_V4L2_SUBDEV_API
mbus_fmt = v4l2_subdev_get_try_format(sd, cfg, 0);
+ if (IS_ERR(mbus_fmt))
+ return PTR_ERR(mbus_fmt);
format->format = *mbus_fmt;
return 0;
-#else
- return -ENOTTY;
-#endif
} else {
format->format = info->format;
}
--
2.19.1
[PATCH 5/6] media: ov5695: get rid of extra ifdefs
Stubbed v4l2_subdev_get_try_format() will return a correct error when
configured without CONFIG_VIDEO_V4L2_SUBDEV_API.
Signed-off-by: Lubomir Rintel
---
drivers/media/i2c/ov5695.c | 38 --
1 file changed, 20 insertions(+), 18 deletions(-)
diff --git a/drivers/media/i2c/ov5695.c b/drivers/media/i2c/ov5695.c
index 5d107c53364d..1469e8b90e1a 100644
--- a/drivers/media/i2c/ov5695.c
+++ b/drivers/media/i2c/ov5695.c
@@ -810,6 +810,7 @@ static int ov5695_set_fmt(struct v4l2_subdev *sd,
struct v4l2_subdev_format *fmt)
{
struct ov5695 *ov5695 = to_ov5695(sd);
+ struct v4l2_mbus_framefmt *try_fmt;
const struct ov5695_mode *mode;
s64 h_blank, vblank_def;
@@ -821,12 +822,12 @@ static int ov5695_set_fmt(struct v4l2_subdev *sd,
fmt->format.height = mode->height;
fmt->format.field = V4L2_FIELD_NONE;
if (fmt->which == V4L2_SUBDEV_FORMAT_TRY) {
-#ifdef CONFIG_VIDEO_V4L2_SUBDEV_API
- *v4l2_subdev_get_try_format(sd, cfg, fmt->pad) = fmt->format;
-#else
- mutex_unlock(&ov5695->mutex);
- return -ENOTTY;
-#endif
+ try_fmt = v4l2_subdev_get_try_format(sd, cfg, fmt->pad);
+ if (IS_ERR(try_fmt)) {
+ mutex_unlock(&ov5695->mutex);
+ return PTR_ERR(try_fmt);
+ }
+ *try_fmt = fmt->format;
} else {
ov5695->cur_mode = mode;
h_blank = mode->hts_def - mode->width;
@@ -845,24 +846,25 @@ static int ov5695_set_fmt(struct v4l2_subdev *sd,
static int ov5695_get_fmt(struct v4l2_subdev *sd,
struct v4l2_subdev_pad_config *cfg,
- struct v4l2_subdev_format *fmt)
+ struct v4l2_subdev_format *format)
{
struct ov5695 *ov5695 = to_ov5695(sd);
+ struct v4l2_mbus_framefmt *fmt;
const struct ov5695_mode *mode = ov5695->cur_mode;
mutex_lock(&ov5695->mutex);
- if (fmt->which == V4L2_SUBDEV_FORMAT_TRY) {
-#ifdef CONFIG_VIDEO_V4L2_SUBDEV_API
- fmt->format = *v4l2_subdev_get_try_format(sd, cfg, fmt->pad);
-#else
- mutex_unlock(&ov5695->mutex);
- return -ENOTTY;
-#endif
+ if (format->which == V4L2_SUBDEV_FORMAT_TRY) {
+ fmt = v4l2_subdev_get_try_format(sd, cfg, format->pad);
+ if (IS_ERR(fmt)) {
+ mutex_unlock(&ov5695->mutex);
+ return PTR_ERR(fmt);
+ }
+ format->format = *fmt;
} else {
- fmt->format.width = mode->width;
- fmt->format.height = mode->height;
- fmt->format.code = MEDIA_BUS_FMT_SBGGR10_1X10;
- fmt->format.field = V4L2_FIELD_NONE;
+ format->format.width = mode->width;
+ format->format.height = mode->height;
+ format->format.code = MEDIA_BUS_FMT_SBGGR10_1X10;
+ format->format.field = V4L2_FIELD_NONE;
}
mutex_unlock(&ov5695->mutex);
--
2.19.1
[PATCH 0/6] media: don't ifdef v4l2_subdev_get_try_format() any more
Hi, this patch set does a minor cleanup as suggested by Jacopo. All driver patches (2-6) depend on the first one. I've just compile-tested most of these as the only actual sensor I have is the ov7670. Cheers, Lubo In-Reply-To:
Re: [PATCH v3 01/14] media: ov7670: split register setting from set_fmt() logic
On Thu, 2018-11-22 at 19:37 +0100, jacopo mondi wrote:
> Hi Lubomir,
>
> On Tue, Nov 20, 2018 at 11:03:06AM +0100, Lubomir Rintel wrote:
> > This will allow us to restore the last set format after the device
> > returns
> > from a power off.
> >
> > Signed-off-by: Lubomir Rintel
> >
> > ---
> > Changes since v2:
> > - This patch was added to the series
> >
> > drivers/media/i2c/ov7670.c | 80 ++--
> > --
> > 1 file changed, 46 insertions(+), 34 deletions(-)
> >
> > diff --git a/drivers/media/i2c/ov7670.c
> > b/drivers/media/i2c/ov7670.c
> > index bc68a3a5b4ec..ee2302fbdeee 100644
> > --- a/drivers/media/i2c/ov7670.c
> > +++ b/drivers/media/i2c/ov7670.c
> > @@ -240,6 +240,7 @@ struct ov7670_info {
> > };
> > struct v4l2_mbus_framefmt format;
> > struct ov7670_format_struct *fmt; /* Current format */
> > + struct ov7670_win_size *wsize;
> > struct clk *clk;
> > struct gpio_desc *resetb_gpio;
> > struct gpio_desc *pwdn_gpio;
> > @@ -1003,48 +1004,20 @@ static int ov7670_try_fmt_internal(struct
> > v4l2_subdev *sd,
> > return 0;
> > }
> >
> > -/*
> > - * Set a format.
> > - */
> > -static int ov7670_set_fmt(struct v4l2_subdev *sd,
> > - struct v4l2_subdev_pad_config *cfg,
> > - struct v4l2_subdev_format *format)
> > +static int ov7670_apply_fmt(struct v4l2_subdev *sd)
> > {
> > - struct ov7670_format_struct *ovfmt;
> > - struct ov7670_win_size *wsize;
> > struct ov7670_info *info = to_state(sd);
> > -#ifdef CONFIG_VIDEO_V4L2_SUBDEV_API
> > - struct v4l2_mbus_framefmt *mbus_fmt;
> > -#endif
> > + struct ov7670_win_size *wsize = info->wsize;
> > unsigned char com7, com10 = 0;
> > int ret;
> >
> > - if (format->pad)
> > - return -EINVAL;
> > -
> > - if (format->which == V4L2_SUBDEV_FORMAT_TRY) {
> > - ret = ov7670_try_fmt_internal(sd, &format->format,
> > NULL, NULL);
> > - if (ret)
> > - return ret;
> > -#ifdef CONFIG_VIDEO_V4L2_SUBDEV_API
> > - mbus_fmt = v4l2_subdev_get_try_format(sd, cfg, format-
> > >pad);
> > - *mbus_fmt = format->format;
> > - return 0;
> > -#else
> > - return -ENOTTY;
> > -#endif
> > - }
> > -
> > - ret = ov7670_try_fmt_internal(sd, &format->format, &ovfmt,
> > &wsize);
> > - if (ret)
> > - return ret;
> > /*
> > * COM7 is a pain in the ass, it doesn't like to be read then
> > * quickly written afterward. But we have everything we need
> > * to set it absolutely here, as long as the format-specific
> > * register sets list it first.
> > */
> > - com7 = ovfmt->regs[0].value;
> > + com7 = info->fmt->regs[0].value;
> > com7 |= wsize->com7_bit;
> > ret = ov7670_write(sd, REG_COM7, com7);
> > if (ret)
> > @@ -1066,7 +1039,7 @@ static int ov7670_set_fmt(struct v4l2_subdev
> > *sd,
> > /*
> > * Now write the rest of the array. Also store start/stops
> > */
> > - ret = ov7670_write_array(sd, ovfmt->regs + 1);
> > + ret = ov7670_write_array(sd, info->fmt->regs + 1);
> > if (ret)
> > return ret;
> >
> > @@ -1081,8 +1054,6 @@ static int ov7670_set_fmt(struct v4l2_subdev
> > *sd,
> > return ret;
> > }
> >
> > - info->fmt = ovfmt;
> > -
> > /*
> > * If we're running RGB565, we must rewrite clkrc after setting
> > * the other parameters or the image looks poor. If we're
> > *not*
> > @@ -1100,6 +1071,46 @@ static int ov7670_set_fmt(struct v4l2_subdev
> > *sd,
> > return 0;
> > }
> >
> > +/*
> > + * Set a format.
> > + */
> > +static int ov7670_set_fmt(struct v4l2_subdev *sd,
> > + struct v4l2_subdev_pad_config *cfg,
> > + struct v4l2_subdev_format *format)
> > +{
> > + struct ov7670_info *info = to_state(sd);
> > +#ifdef CONFIG_VIDEO_V4L2_SUBDEV_API
> > + struct v4l2_mbus_framefmt *mbus_fmt;
> > +#endif
> > + int ret;
> > +
> > + if (format->pad)
> > + return -EINVAL;
> > +
> > + if (format->which == V4L2_SUBDEV_FORMAT_TRY) {
> > + ret = ov7670_try_fmt_inte
Re: [PATCH v3 03/14] media: ov7670: hook s_power onto v4l2 core
On Thu, 2018-11-22 at 14:21 +0200, Sakari Ailus wrote:
> Hi Lubomir,
>
> On Tue, Nov 20, 2018 at 11:03:08AM +0100, Lubomir Rintel wrote:
> > The commit 71862f63f351 ("media: ov7670: Add the ov7670_s_power function")
> > added a power control routing. However, it was not good enough to use as
> > a s_power() callback: it merely flipped on the power GPIOs without
> > restoring the register settings.
> >
> > Fix this now and register an actual power callback.
> >
> > Signed-off-by: Lubomir Rintel
> >
> > ---
> > Changes since v2:
> > - Restore the controls, format and frame rate on power on
> >
> > drivers/media/i2c/ov7670.c | 50 +-
> > 1 file changed, 44 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/media/i2c/ov7670.c b/drivers/media/i2c/ov7670.c
> > index ead0c360df33..cbaab60c 100644
> > --- a/drivers/media/i2c/ov7670.c
> > +++ b/drivers/media/i2c/ov7670.c
> > @@ -242,6 +242,7 @@ struct ov7670_info {
> > struct ov7670_format_struct *fmt; /* Current format */
> > struct ov7670_win_size *wsize;
> > struct clk *clk;
> > + int on;
> > struct gpio_desc *resetb_gpio;
> > struct gpio_desc *pwdn_gpio;
> > unsigned int mbus_config; /* Media bus configuration flags */
> > @@ -1615,19 +1616,54 @@ static int ov7670_s_register(struct v4l2_subdev
> > *sd, const struct v4l2_dbg_regis
> > }
> > #endif
> >
> > -static int ov7670_s_power(struct v4l2_subdev *sd, int on)
> > +static void ov7670_power_on(struct v4l2_subdev *sd)
> > {
> > struct ov7670_info *info = to_state(sd);
> >
> > + if (info->on)
> > + return;
> > +
> > if (info->pwdn_gpio)
> > - gpiod_set_value(info->pwdn_gpio, !on);
> > - if (on && info->resetb_gpio) {
> > + gpiod_set_value(info->pwdn_gpio, 0);
> > + if (info->resetb_gpio) {
> > gpiod_set_value(info->resetb_gpio, 1);
> > usleep_range(500, 1000);
> > gpiod_set_value(info->resetb_gpio, 0);
> > usleep_range(3000, 5000);
> > }
> >
> > + info->on = true;
> > +}
> > +
> > +static void ov7670_power_off(struct v4l2_subdev *sd)
> > +{
> > + struct ov7670_info *info = to_state(sd);
> > +
> > + if (!info->on)
> > + return;
> > +
> > + if (info->pwdn_gpio)
> > + gpiod_set_value(info->pwdn_gpio, 1);
> > +
> > + info->on = false;
> > +}
> > +
> > +static int ov7670_s_power(struct v4l2_subdev *sd, int on)
> > +{
> > + struct ov7670_info *info = to_state(sd);
> > +
> > + if (info->on == on)
> > + return 0;
> > +
> > + if (on) {
> > + ov7670_power_on (sd);
> > + ov7670_apply_fmt(sd);
> > + ov7675_apply_framerate(sd);
> > + v4l2_ctrl_handler_setup(&info->hdl);
> > + } else {
> > + ov7670_power_off (sd);
> > + }
> > +
> > return 0;
> > }
> >
> > @@ -1660,6 +1696,7 @@ static int ov7670_open(struct v4l2_subdev *sd, struct
> > v4l2_subdev_fh *fh)
> > static const struct v4l2_subdev_core_ops ov7670_core_ops = {
> > .reset = ov7670_reset,
> > .init = ov7670_init,
> > + .s_power = ov7670_s_power,
> > #ifdef CONFIG_VIDEO_ADV_DEBUG
> > .g_register = ov7670_g_register,
> > .s_register = ov7670_s_register,
> > @@ -1825,6 +1862,7 @@ static int ov7670_probe(struct i2c_client *client,
> > else
> > return ret;
> > }
> > +
> > if (info->clk) {
> > ret = clk_prepare_enable(info->clk);
> > if (ret)
> > @@ -1841,7 +1879,7 @@ static int ov7670_probe(struct i2c_client *client,
> > if (ret)
> > goto clk_disable;
> >
> > - ov7670_s_power(sd, 1);
> > + ov7670_power_on(sd);
> >
> > /* Make sure it's an ov7670 */
> > ret = ov7670_detect(sd);
> > @@ -1929,7 +1967,7 @@ static int ov7670_probe(struct i2c_client *client,
> > hdl_free:
> > v4l2_ctrl_handler_free(&info->hdl);
> > power_off:
> > - ov7670_s_power(sd, 0);
> > + ov7670_power_off(sd);
> > clk_disable:
> > clk_disable_unprepare(info->clk);
> > return ret;
> > @@ -1945,7 +1983,7 @@ static int ov7670_remove(struct i2c_client *client)
> > v4l2_ctrl_handler_free(&info->hdl);
> > clk_disable_unprepare(info->clk);
> > media_entity_cleanup(&info->sd.entity);
> > - ov7670_s_power(sd, 0);
> > + ov7670_power_off(sd);
> > return 0;
> > }
> >
>
> Could you consider instead switching to runtime PM? A few drivers such as
> the ov2685 driver does that already.
Yes, I'll take a look. Thanks for the suggestion. I didn't know such
thing exists, so it may take some time for me to grasp it though.
Take care,
Lubo
>
[PATCH v3 01/14] media: ov7670: split register setting from set_fmt() logic
This will allow us to restore the last set format after the device returns
from a power off.
Signed-off-by: Lubomir Rintel
---
Changes since v2:
- This patch was added to the series
drivers/media/i2c/ov7670.c | 80 ++
1 file changed, 46 insertions(+), 34 deletions(-)
diff --git a/drivers/media/i2c/ov7670.c b/drivers/media/i2c/ov7670.c
index bc68a3a5b4ec..ee2302fbdeee 100644
--- a/drivers/media/i2c/ov7670.c
+++ b/drivers/media/i2c/ov7670.c
@@ -240,6 +240,7 @@ struct ov7670_info {
};
struct v4l2_mbus_framefmt format;
struct ov7670_format_struct *fmt; /* Current format */
+ struct ov7670_win_size *wsize;
struct clk *clk;
struct gpio_desc *resetb_gpio;
struct gpio_desc *pwdn_gpio;
@@ -1003,48 +1004,20 @@ static int ov7670_try_fmt_internal(struct v4l2_subdev
*sd,
return 0;
}
-/*
- * Set a format.
- */
-static int ov7670_set_fmt(struct v4l2_subdev *sd,
- struct v4l2_subdev_pad_config *cfg,
- struct v4l2_subdev_format *format)
+static int ov7670_apply_fmt(struct v4l2_subdev *sd)
{
- struct ov7670_format_struct *ovfmt;
- struct ov7670_win_size *wsize;
struct ov7670_info *info = to_state(sd);
-#ifdef CONFIG_VIDEO_V4L2_SUBDEV_API
- struct v4l2_mbus_framefmt *mbus_fmt;
-#endif
+ struct ov7670_win_size *wsize = info->wsize;
unsigned char com7, com10 = 0;
int ret;
- if (format->pad)
- return -EINVAL;
-
- if (format->which == V4L2_SUBDEV_FORMAT_TRY) {
- ret = ov7670_try_fmt_internal(sd, &format->format, NULL, NULL);
- if (ret)
- return ret;
-#ifdef CONFIG_VIDEO_V4L2_SUBDEV_API
- mbus_fmt = v4l2_subdev_get_try_format(sd, cfg, format->pad);
- *mbus_fmt = format->format;
- return 0;
-#else
- return -ENOTTY;
-#endif
- }
-
- ret = ov7670_try_fmt_internal(sd, &format->format, &ovfmt, &wsize);
- if (ret)
- return ret;
/*
* COM7 is a pain in the ass, it doesn't like to be read then
* quickly written afterward. But we have everything we need
* to set it absolutely here, as long as the format-specific
* register sets list it first.
*/
- com7 = ovfmt->regs[0].value;
+ com7 = info->fmt->regs[0].value;
com7 |= wsize->com7_bit;
ret = ov7670_write(sd, REG_COM7, com7);
if (ret)
@@ -1066,7 +1039,7 @@ static int ov7670_set_fmt(struct v4l2_subdev *sd,
/*
* Now write the rest of the array. Also store start/stops
*/
- ret = ov7670_write_array(sd, ovfmt->regs + 1);
+ ret = ov7670_write_array(sd, info->fmt->regs + 1);
if (ret)
return ret;
@@ -1081,8 +1054,6 @@ static int ov7670_set_fmt(struct v4l2_subdev *sd,
return ret;
}
- info->fmt = ovfmt;
-
/*
* If we're running RGB565, we must rewrite clkrc after setting
* the other parameters or the image looks poor. If we're *not*
@@ -1100,6 +1071,46 @@ static int ov7670_set_fmt(struct v4l2_subdev *sd,
return 0;
}
+/*
+ * Set a format.
+ */
+static int ov7670_set_fmt(struct v4l2_subdev *sd,
+ struct v4l2_subdev_pad_config *cfg,
+ struct v4l2_subdev_format *format)
+{
+ struct ov7670_info *info = to_state(sd);
+#ifdef CONFIG_VIDEO_V4L2_SUBDEV_API
+ struct v4l2_mbus_framefmt *mbus_fmt;
+#endif
+ int ret;
+
+ if (format->pad)
+ return -EINVAL;
+
+ if (format->which == V4L2_SUBDEV_FORMAT_TRY) {
+ ret = ov7670_try_fmt_internal(sd, &format->format, NULL, NULL);
+ if (ret)
+ return ret;
+#ifdef CONFIG_VIDEO_V4L2_SUBDEV_API
+ mbus_fmt = v4l2_subdev_get_try_format(sd, cfg, format->pad);
+ *mbus_fmt = format->format;
+ return 0;
+#else
+ return -ENOTTY;
+#endif
+ }
+
+ ret = ov7670_try_fmt_internal(sd, &format->format, &info->fmt,
&info->wsize);
+ if (ret)
+ return ret;
+
+ ret = ov7670_apply_fmt(sd);
+ if (ret)
+ return ret;
+
+ return 0;
+}
+
static int ov7670_get_fmt(struct v4l2_subdev *sd,
struct v4l2_subdev_pad_config *cfg,
struct v4l2_subdev_format *format)
@@ -1847,6 +1858,7 @@ static int ov7670_probe(struct i2c_client *client,
info->devtype = &ov7670_devdata[id->driver_data];
info->fmt = &ov7670_formats[0];
+ info->wsize = &info->devtype->win_sizes[0];
ov7670_get_default_format(sd, &info->format);
--
2.19.1
[PATCH v3 04/14] media: ov7670: control clock along with power
This provides more power saving when the sensor is off.
While at that, do the delay on power/clock enable even if the sensor driver
itself doesn't control the GPIOs. This is required for the OLPC XO-1
platform, that lacks the proper power/reset properties in its DT, but
needs the delay after the sensor is clocked up.
Signed-off-by: Lubomir Rintel
---
drivers/media/i2c/ov7670.c | 30 ++
1 file changed, 14 insertions(+), 16 deletions(-)
diff --git a/drivers/media/i2c/ov7670.c b/drivers/media/i2c/ov7670.c
index cbaab60c..d7635fb2d713 100644
--- a/drivers/media/i2c/ov7670.c
+++ b/drivers/media/i2c/ov7670.c
@@ -1623,14 +1623,17 @@ static void ov7670_power_on(struct v4l2_subdev *sd)
if (info->on)
return;
+ clk_prepare_enable(info->clk);
+
if (info->pwdn_gpio)
gpiod_set_value(info->pwdn_gpio, 0);
if (info->resetb_gpio) {
gpiod_set_value(info->resetb_gpio, 1);
usleep_range(500, 1000);
gpiod_set_value(info->resetb_gpio, 0);
- usleep_range(3000, 5000);
}
+ if (info->pwdn_gpio || info->resetb_gpio || info->clk)
+ usleep_range(3000, 5000);
info->on = true;
}
@@ -1642,6 +1645,8 @@ static void ov7670_power_off(struct v4l2_subdev *sd)
if (!info->on)
return;
+ clk_disable_unprepare(info->clk);
+
if (info->pwdn_gpio)
gpiod_set_value(info->pwdn_gpio, 1);
@@ -1863,24 +1868,20 @@ static int ov7670_probe(struct i2c_client *client,
return ret;
}
- if (info->clk) {
- ret = clk_prepare_enable(info->clk);
- if (ret)
- return ret;
+ ret = ov7670_init_gpio(client, info);
+ if (ret)
+ return ret;
+ ov7670_power_on(sd);
+
+ if (info->clk) {
info->clock_speed = clk_get_rate(info->clk) / 100;
if (info->clock_speed < 10 || info->clock_speed > 48) {
ret = -EINVAL;
- goto clk_disable;
+ goto power_off;
}
}
- ret = ov7670_init_gpio(client, info);
- if (ret)
- goto clk_disable;
-
- ov7670_power_on(sd);
-
/* Make sure it's an ov7670 */
ret = ov7670_detect(sd);
if (ret) {
@@ -1960,6 +1961,7 @@ static int ov7670_probe(struct i2c_client *client,
if (ret < 0)
goto entity_cleanup;
+ ov7670_power_off(sd);
return 0;
entity_cleanup:
@@ -1968,12 +1970,9 @@ static int ov7670_probe(struct i2c_client *client,
v4l2_ctrl_handler_free(&info->hdl);
power_off:
ov7670_power_off(sd);
-clk_disable:
- clk_disable_unprepare(info->clk);
return ret;
}
-
static int ov7670_remove(struct i2c_client *client)
{
struct v4l2_subdev *sd = i2c_get_clientdata(client);
@@ -1981,7 +1980,6 @@ static int ov7670_remove(struct i2c_client *client)
v4l2_async_unregister_subdev(sd);
v4l2_ctrl_handler_free(&info->hdl);
- clk_disable_unprepare(info->clk);
media_entity_cleanup(&info->sd.entity);
ov7670_power_off(sd);
return 0;
--
2.19.1
[PATCH v3 05/14] media: dt-bindings: marvell,mmp2-ccic: Add Marvell MMP2 camera
Add Marvell MMP2 camera host interface.
Signed-off-by: Lubomir Rintel
---
Changes since v2:
- Added #clock-cells, clock-names, port
.../bindings/media/marvell,mmp2-ccic.txt | 37 +++
1 file changed, 37 insertions(+)
create mode 100644
Documentation/devicetree/bindings/media/marvell,mmp2-ccic.txt
diff --git a/Documentation/devicetree/bindings/media/marvell,mmp2-ccic.txt
b/Documentation/devicetree/bindings/media/marvell,mmp2-ccic.txt
new file mode 100644
index ..e5e8ca90e7f7
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/marvell,mmp2-ccic.txt
@@ -0,0 +1,37 @@
+Marvell MMP2 camera host interface
+
+Required properties:
+ - compatible: Should be "marvell,mmp2-ccic"
+ - reg: register base and size
+ - interrupts: the interrupt number
+ - #clock-cells: must be 0
+ - any required generic properties defined in video-interfaces.txt
+
+Optional properties:
+ - clocks: input clock (see clock-bindings.txt)
+ - clock-names: names of the clocks used, may include "axi", "func" and
+"phy"
+ - clock-output-names: should contain the name of the clock driving the
+ sensor master clock MCLK
+
+Required subnodes:
+ - port: the parallel bus interface port with a single endpoint linked to
+ the sensor's endpoint as described in video-interfaces.txt
+
+Example:
+
+ camera0: camera@d420a000 {
+ compatible = "marvell,mmp2-ccic";
+ reg = <0xd420a000 0x800>;
+ interrupts = <42>;
+ clocks = <&soc_clocks MMP2_CLK_CCIC0>;
+ clock-names = "axi";
+ #clock-cells = <0>;
+ clock-output-names = "mclk";
+
+ port {
+ camera0_0: endpoint {
+ remote-endpoint = <&ov7670_0>;
+ };
+ };
+ };
--
2.19.1
[PATCH v3 03/14] media: ov7670: hook s_power onto v4l2 core
The commit 71862f63f351 ("media: ov7670: Add the ov7670_s_power function")
added a power control routing. However, it was not good enough to use as
a s_power() callback: it merely flipped on the power GPIOs without
restoring the register settings.
Fix this now and register an actual power callback.
Signed-off-by: Lubomir Rintel
---
Changes since v2:
- Restore the controls, format and frame rate on power on
drivers/media/i2c/ov7670.c | 50 +-
1 file changed, 44 insertions(+), 6 deletions(-)
diff --git a/drivers/media/i2c/ov7670.c b/drivers/media/i2c/ov7670.c
index ead0c360df33..cbaab60c 100644
--- a/drivers/media/i2c/ov7670.c
+++ b/drivers/media/i2c/ov7670.c
@@ -242,6 +242,7 @@ struct ov7670_info {
struct ov7670_format_struct *fmt; /* Current format */
struct ov7670_win_size *wsize;
struct clk *clk;
+ int on;
struct gpio_desc *resetb_gpio;
struct gpio_desc *pwdn_gpio;
unsigned int mbus_config; /* Media bus configuration flags */
@@ -1615,19 +1616,54 @@ static int ov7670_s_register(struct v4l2_subdev *sd,
const struct v4l2_dbg_regis
}
#endif
-static int ov7670_s_power(struct v4l2_subdev *sd, int on)
+static void ov7670_power_on(struct v4l2_subdev *sd)
{
struct ov7670_info *info = to_state(sd);
+ if (info->on)
+ return;
+
if (info->pwdn_gpio)
- gpiod_set_value(info->pwdn_gpio, !on);
- if (on && info->resetb_gpio) {
+ gpiod_set_value(info->pwdn_gpio, 0);
+ if (info->resetb_gpio) {
gpiod_set_value(info->resetb_gpio, 1);
usleep_range(500, 1000);
gpiod_set_value(info->resetb_gpio, 0);
usleep_range(3000, 5000);
}
+ info->on = true;
+}
+
+static void ov7670_power_off(struct v4l2_subdev *sd)
+{
+ struct ov7670_info *info = to_state(sd);
+
+ if (!info->on)
+ return;
+
+ if (info->pwdn_gpio)
+ gpiod_set_value(info->pwdn_gpio, 1);
+
+ info->on = false;
+}
+
+static int ov7670_s_power(struct v4l2_subdev *sd, int on)
+{
+ struct ov7670_info *info = to_state(sd);
+
+ if (info->on == on)
+ return 0;
+
+ if (on) {
+ ov7670_power_on (sd);
+ ov7670_apply_fmt(sd);
+ ov7675_apply_framerate(sd);
+ v4l2_ctrl_handler_setup(&info->hdl);
+ } else {
+ ov7670_power_off (sd);
+ }
+
return 0;
}
@@ -1660,6 +1696,7 @@ static int ov7670_open(struct v4l2_subdev *sd, struct
v4l2_subdev_fh *fh)
static const struct v4l2_subdev_core_ops ov7670_core_ops = {
.reset = ov7670_reset,
.init = ov7670_init,
+ .s_power = ov7670_s_power,
#ifdef CONFIG_VIDEO_ADV_DEBUG
.g_register = ov7670_g_register,
.s_register = ov7670_s_register,
@@ -1825,6 +1862,7 @@ static int ov7670_probe(struct i2c_client *client,
else
return ret;
}
+
if (info->clk) {
ret = clk_prepare_enable(info->clk);
if (ret)
@@ -1841,7 +1879,7 @@ static int ov7670_probe(struct i2c_client *client,
if (ret)
goto clk_disable;
- ov7670_s_power(sd, 1);
+ ov7670_power_on(sd);
/* Make sure it's an ov7670 */
ret = ov7670_detect(sd);
@@ -1929,7 +1967,7 @@ static int ov7670_probe(struct i2c_client *client,
hdl_free:
v4l2_ctrl_handler_free(&info->hdl);
power_off:
- ov7670_s_power(sd, 0);
+ ov7670_power_off(sd);
clk_disable:
clk_disable_unprepare(info->clk);
return ret;
@@ -1945,7 +1983,7 @@ static int ov7670_remove(struct i2c_client *client)
v4l2_ctrl_handler_free(&info->hdl);
clk_disable_unprepare(info->clk);
media_entity_cleanup(&info->sd.entity);
- ov7670_s_power(sd, 0);
+ ov7670_power_off(sd);
return 0;
}
--
2.19.1
[PATCH v3 07/14] [media] marvell-ccic: don't generate EOF on parallel bus
The commit 05fed81625bf ("[media] marvell-ccic: add MIPI support for
marvell-ccic driver") that claimed to add CSI2 turned on C0_EOF_VSYNC for
parallel bus without a very good explanation.
That broke camera on OLPC XO-1.75 which precisely uses a sensor on a
parallel bus. Revert that chunk.
Tested on an OLPC XO-1.75.
Fixes: 05fed81625bf755cc67c5864cdfd18b69ea828d1
Signed-off-by: Lubomir Rintel
---
drivers/media/platform/marvell-ccic/mcam-core.c | 6 --
1 file changed, 6 deletions(-)
diff --git a/drivers/media/platform/marvell-ccic/mcam-core.c
b/drivers/media/platform/marvell-ccic/mcam-core.c
index d97f39bde9bd..d24e5b7a3bc5 100644
--- a/drivers/media/platform/marvell-ccic/mcam-core.c
+++ b/drivers/media/platform/marvell-ccic/mcam-core.c
@@ -792,12 +792,6 @@ static void mcam_ctlr_image(struct mcam_camera *cam)
* Make sure it knows we want to use hsync/vsync.
*/
mcam_reg_write_mask(cam, REG_CTRL0, C0_SIF_HVSYNC, C0_SIFM_MASK);
- /*
-* This field controls the generation of EOF(DVP only)
-*/
- if (cam->bus_type != V4L2_MBUS_CSI2_DPHY)
- mcam_reg_set_bit(cam, REG_CTRL0,
- C0_EOF_VSYNC | C0_VEDGE_CTRL);
}
--
2.19.1
[PATCH v3 06/14] [media] marvell-ccic: fix DMA s/g desc number calculation
The commit d790b7eda953 ("[media] vb2-dma-sg: move dma_(un)map_sg here")
left dma_desc_nent unset. It previously contained the number of DMA
descriptors as returned from dma_map_sg().
We can now (since the commit referred to above) obtain the same value from
the sg_table and drop dma_desc_nent altogether.
Tested on OLPC XO-1.75 machine. Doesn't affect the OLPC XO-1's Cafe
driver, since that one doesn't do DMA.
Fixes: d790b7eda953df474f470169ebdf111c02fa7a2d
Signed-off-by: Lubomir Rintel
---
drivers/media/platform/marvell-ccic/mcam-core.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/media/platform/marvell-ccic/mcam-core.c
b/drivers/media/platform/marvell-ccic/mcam-core.c
index f1b301810260..d97f39bde9bd 100644
--- a/drivers/media/platform/marvell-ccic/mcam-core.c
+++ b/drivers/media/platform/marvell-ccic/mcam-core.c
@@ -200,7 +200,6 @@ struct mcam_vb_buffer {
struct list_head queue;
struct mcam_dma_desc *dma_desc; /* Descriptor virtual address */
dma_addr_t dma_desc_pa; /* Descriptor physical address */
- int dma_desc_nent; /* Number of mapped descriptors */
};
static inline struct mcam_vb_buffer *vb_to_mvb(struct vb2_v4l2_buffer *vb)
@@ -608,9 +607,11 @@ static void mcam_dma_contig_done(struct mcam_camera *cam,
int frame)
static void mcam_sg_next_buffer(struct mcam_camera *cam)
{
struct mcam_vb_buffer *buf;
+ struct sg_table *sg_table;
buf = list_first_entry(&cam->buffers, struct mcam_vb_buffer, queue);
list_del_init(&buf->queue);
+ sg_table = vb2_dma_sg_plane_desc(&buf->vb_buf.vb2_buf, 0);
/*
* Very Bad Not Good Things happen if you don't clear
* C1_DESC_ENA before making any descriptor changes.
@@ -618,7 +619,7 @@ static void mcam_sg_next_buffer(struct mcam_camera *cam)
mcam_reg_clear_bit(cam, REG_CTRL1, C1_DESC_ENA);
mcam_reg_write(cam, REG_DMA_DESC_Y, buf->dma_desc_pa);
mcam_reg_write(cam, REG_DESC_LEN_Y,
- buf->dma_desc_nent*sizeof(struct mcam_dma_desc));
+ sg_table->nents*sizeof(struct mcam_dma_desc));
mcam_reg_write(cam, REG_DESC_LEN_U, 0);
mcam_reg_write(cam, REG_DESC_LEN_V, 0);
mcam_reg_set_bit(cam, REG_CTRL1, C1_DESC_ENA);
--
2.19.1
[PATCH v3 09/14] [media] marvell-ccic: drop unused stuff
Remove structure members and headers that are not actually used. Saves
us from some noise in subsequent cleanup commits.
Signed-off-by: Lubomir Rintel
Acked-by: Pavel Machek
---
drivers/media/platform/marvell-ccic/mcam-core.c | 1 -
drivers/media/platform/marvell-ccic/mcam-core.h | 2 --
drivers/media/platform/marvell-ccic/mmp-driver.c | 2 --
include/linux/platform_data/media/mmp-camera.h | 1 -
4 files changed, 6 deletions(-)
diff --git a/drivers/media/platform/marvell-ccic/mcam-core.c
b/drivers/media/platform/marvell-ccic/mcam-core.c
index 1b879035948c..0113b8d37d03 100644
--- a/drivers/media/platform/marvell-ccic/mcam-core.c
+++ b/drivers/media/platform/marvell-ccic/mcam-core.c
@@ -1776,7 +1776,6 @@ int mccic_register(struct mcam_camera *cam)
*/
sensor_cfg.clock_speed = cam->clock_speed;
sensor_cfg.use_smbus = cam->use_smbus;
- cam->sensor_addr = ov7670_info.addr;
cam->sensor = v4l2_i2c_new_subdev_board(&cam->v4l2_dev,
cam->i2c_adapter, &ov7670_info, NULL);
if (cam->sensor == NULL) {
diff --git a/drivers/media/platform/marvell-ccic/mcam-core.h
b/drivers/media/platform/marvell-ccic/mcam-core.h
index a3a097a45e78..b828b1bb59d3 100644
--- a/drivers/media/platform/marvell-ccic/mcam-core.h
+++ b/drivers/media/platform/marvell-ccic/mcam-core.h
@@ -112,7 +112,6 @@ struct mcam_camera {
short int use_smbus;/* SMBUS or straight I2c? */
enum mcam_buffer_mode buffer_mode;
- int mclk_min; /* The minimal value of mclk */
int mclk_src; /* which clock source the mclk derives from */
int mclk_div; /* Clock Divider Value for MCLK */
@@ -152,7 +151,6 @@ struct mcam_camera {
*/
struct video_device vdev;
struct v4l2_subdev *sensor;
- unsigned short sensor_addr;
/* Videobuf2 stuff */
struct vb2_queue vb_queue;
diff --git a/drivers/media/platform/marvell-ccic/mmp-driver.c
b/drivers/media/platform/marvell-ccic/mmp-driver.c
index dbfc597b955d..f2e43b23af18 100644
--- a/drivers/media/platform/marvell-ccic/mmp-driver.c
+++ b/drivers/media/platform/marvell-ccic/mmp-driver.c
@@ -12,7 +12,6 @@
#include
#include
#include
-#include
#include
#include
#include
@@ -332,7 +331,6 @@ static int mmpcam_probe(struct platform_device *pdev)
mcam->calc_dphy = mmpcam_calc_dphy;
mcam->dev = &pdev->dev;
mcam->use_smbus = 0;
- mcam->mclk_min = pdata->mclk_min;
mcam->mclk_src = pdata->mclk_src;
mcam->mclk_div = pdata->mclk_div;
mcam->bus_type = pdata->bus_type;
diff --git a/include/linux/platform_data/media/mmp-camera.h
b/include/linux/platform_data/media/mmp-camera.h
index d2d3a443eedf..4c3a80a45883 100644
--- a/include/linux/platform_data/media/mmp-camera.h
+++ b/include/linux/platform_data/media/mmp-camera.h
@@ -16,7 +16,6 @@ struct mmp_camera_platform_data {
int sensor_power_gpio;
int sensor_reset_gpio;
enum v4l2_mbus_type bus_type;
- int mclk_min; /* The minimal value of MCLK */
int mclk_src; /* which clock source the MCLK derives from */
int mclk_div; /* Clock Divider Value for MCLK */
/*
--
2.19.1
[PATCH v3 11/14] [media] marvell-ccic: rename the clocks
Use the names more suitable for devicetree bindings.
There are no board files utilizing this, thus we seem to be at liberty
at renaming this without consequences.
Signed-off-by: Lubomir Rintel
---
Changes since v2:
- This patch was added to the series
drivers/media/platform/marvell-ccic/mmp-driver.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/media/platform/marvell-ccic/mmp-driver.c
b/drivers/media/platform/marvell-ccic/mmp-driver.c
index 05cba74c0d13..5c61317f56bc 100644
--- a/drivers/media/platform/marvell-ccic/mmp-driver.c
+++ b/drivers/media/platform/marvell-ccic/mmp-driver.c
@@ -33,7 +33,7 @@ MODULE_ALIAS("platform:mmp-camera");
MODULE_AUTHOR("Jonathan Corbet ");
MODULE_LICENSE("GPL");
-static char *mcam_clks[] = {"CCICAXICLK", "CCICFUNCLK", "CCICPHYCLK"};
+static char *mcam_clks[] = {"axi", "func", "phy"};
struct mmp_camera {
void __iomem *power_regs;
--
2.19.1
[PATCH v3 08/14] Revert "[media] marvell-ccic: reset ccic phy when stop streaming for stability"
This accesses the clock registers directly and thus is going to stay in the
way of making the driver devicetree friendly.
No boards seems to actually use this. If it's somehow actually needed it
needs to be done differently.
This reverts commit 7c269f454e7a51b151d94f99344120efa1cd0acb.
---
.../media/platform/marvell-ccic/mcam-core.c | 6 -
.../media/platform/marvell-ccic/mcam-core.h | 2 --
.../media/platform/marvell-ccic/mmp-driver.c | 25 ---
3 files changed, 33 deletions(-)
diff --git a/drivers/media/platform/marvell-ccic/mcam-core.c
b/drivers/media/platform/marvell-ccic/mcam-core.c
index d24e5b7a3bc5..1b879035948c 100644
--- a/drivers/media/platform/marvell-ccic/mcam-core.c
+++ b/drivers/media/platform/marvell-ccic/mcam-core.c
@@ -1154,12 +1154,6 @@ static void mcam_vb_stop_streaming(struct vb2_queue *vq)
if (cam->state != S_STREAMING)
return;
mcam_ctlr_stop_dma(cam);
- /*
-* Reset the CCIC PHY after stopping streaming,
-* otherwise, the CCIC may be unstable.
-*/
- if (cam->ctlr_reset)
- cam->ctlr_reset(cam);
/*
* VB2 reclaims the buffers, so we need to forget
* about them.
diff --git a/drivers/media/platform/marvell-ccic/mcam-core.h
b/drivers/media/platform/marvell-ccic/mcam-core.h
index ad8955f9f0a1..a3a097a45e78 100644
--- a/drivers/media/platform/marvell-ccic/mcam-core.h
+++ b/drivers/media/platform/marvell-ccic/mcam-core.h
@@ -116,7 +116,6 @@ struct mcam_camera {
int mclk_src; /* which clock source the mclk derives from */
int mclk_div; /* Clock Divider Value for MCLK */
- int ccic_id;
enum v4l2_mbus_type bus_type;
/* MIPI support */
/* The dphy config value, allocated in board file
@@ -137,7 +136,6 @@ struct mcam_camera {
int (*plat_power_up) (struct mcam_camera *cam);
void (*plat_power_down) (struct mcam_camera *cam);
void (*calc_dphy) (struct mcam_camera *cam);
- void (*ctlr_reset) (struct mcam_camera *cam);
/*
* Everything below here is private to the mcam core and
diff --git a/drivers/media/platform/marvell-ccic/mmp-driver.c
b/drivers/media/platform/marvell-ccic/mmp-driver.c
index 70a2833db0d1..dbfc597b955d 100644
--- a/drivers/media/platform/marvell-ccic/mmp-driver.c
+++ b/drivers/media/platform/marvell-ccic/mmp-driver.c
@@ -105,7 +105,6 @@ static struct mmp_camera *mmpcam_find_device(struct
platform_device *pdev)
#define CPU_SUBSYS_PMU_BASE0xd4282800
#define REG_CCIC_DCGCR 0x28/* CCIC dyn clock gate ctrl reg */
#define REG_CCIC_CRCR 0x50/* CCIC clk reset ctrl reg */
-#define REG_CCIC2_CRCR 0xf4/* CCIC2 clk reset ctrl reg */
static void mcam_clk_enable(struct mcam_camera *mcam)
{
@@ -183,28 +182,6 @@ static void mmpcam_power_down(struct mcam_camera *mcam)
mcam_clk_disable(mcam);
}
-static void mcam_ctlr_reset(struct mcam_camera *mcam)
-{
- unsigned long val;
- struct mmp_camera *cam = mcam_to_cam(mcam);
-
- if (mcam->ccic_id) {
- /*
-* Using CCIC2
-*/
- val = ioread32(cam->power_regs + REG_CCIC2_CRCR);
- iowrite32(val & ~0x2, cam->power_regs + REG_CCIC2_CRCR);
- iowrite32(val | 0x2, cam->power_regs + REG_CCIC2_CRCR);
- } else {
- /*
-* Using CCIC1
-*/
- val = ioread32(cam->power_regs + REG_CCIC_CRCR);
- iowrite32(val & ~0x2, cam->power_regs + REG_CCIC_CRCR);
- iowrite32(val | 0x2, cam->power_regs + REG_CCIC_CRCR);
- }
-}
-
/*
* calc the dphy register values
* There are three dphy registers being used.
@@ -352,11 +329,9 @@ static int mmpcam_probe(struct platform_device *pdev)
mcam = &cam->mcam;
mcam->plat_power_up = mmpcam_power_up;
mcam->plat_power_down = mmpcam_power_down;
- mcam->ctlr_reset = mcam_ctlr_reset;
mcam->calc_dphy = mmpcam_calc_dphy;
mcam->dev = &pdev->dev;
mcam->use_smbus = 0;
- mcam->ccic_id = pdev->id;
mcam->mclk_min = pdata->mclk_min;
mcam->mclk_src = pdata->mclk_src;
mcam->mclk_div = pdata->mclk_div;
--
2.19.1
[PATCH v3 10/14] [media] marvell-ccic/mmp: enable clock before accessing registers
The access to REG_CLKCTRL or REG_CTRL1 without the clock enabled hangs the machine. Enable the clock first. Signed-off-by: Lubomir Rintel Acked-by: Pavel Machek --- drivers/media/platform/marvell-ccic/mmp-driver.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/drivers/media/platform/marvell-ccic/mmp-driver.c b/drivers/media/platform/marvell-ccic/mmp-driver.c index f2e43b23af18..05cba74c0d13 100644 --- a/drivers/media/platform/marvell-ccic/mmp-driver.c +++ b/drivers/media/platform/marvell-ccic/mmp-driver.c @@ -144,6 +144,7 @@ static int mmpcam_power_up(struct mcam_camera *mcam) * Turn on power and clocks to the controller. */ mmpcam_power_up_ctlr(cam); + mcam_clk_enable(mcam); /* * Provide power to the sensor. */ @@ -157,8 +158,6 @@ static int mmpcam_power_up(struct mcam_camera *mcam) gpio_set_value(pdata->sensor_reset_gpio, 1); /* reset is active low */ mdelay(5); - mcam_clk_enable(mcam); - return 0; } -- 2.19.1
[PATCH v3 14/14] [media] marvell-ccic: provide a clock for the sensor
The sensor needs the MCLK clock running when it's being probed. On
platforms where the sensor is instantiated from a DT (MMP2) it is going
to happen asynchronously.
Therefore, the current modus operandi, where the bridge driver fiddles
with the sensor power and clock itself is not going to fly. As the comments
wisely note, this doesn't even belong there.
Luckily, the ov7670 driver is already able to control its power and
reset lines, we can just drop the MMP platform glue altogether.
It also requests the clock via the standard clock subsystem. Good -- let's
set up a clock instance so that the sensor can ask us to enable the clock.
Note that this is pretty dumb at the moment: the clock is hardwired to a
particular frequency and parent. It was always the case.
Signed-off-by: Lubomir Rintel
---
Changes since v1:
- [kbuild/ia64] depend on COMMON_CLK.
- [smatch] fix bad return in mcam_v4l_open() leading to lock not getting
released on error.
drivers/media/platform/marvell-ccic/Kconfig | 2 +
.../media/platform/marvell-ccic/cafe-driver.c | 9 +-
.../media/platform/marvell-ccic/mcam-core.c | 156 +++---
.../media/platform/marvell-ccic/mcam-core.h | 3 +
.../media/platform/marvell-ccic/mmp-driver.c | 152 ++---
.../linux/platform_data/media/mmp-camera.h| 2 -
6 files changed, 161 insertions(+), 163 deletions(-)
diff --git a/drivers/media/platform/marvell-ccic/Kconfig
b/drivers/media/platform/marvell-ccic/Kconfig
index cf12e077203a..3e12eb25740a 100644
--- a/drivers/media/platform/marvell-ccic/Kconfig
+++ b/drivers/media/platform/marvell-ccic/Kconfig
@@ -1,6 +1,7 @@
config VIDEO_CAFE_CCIC
tristate "Marvell 88ALP01 (Cafe) CMOS Camera Controller support"
depends on PCI && I2C && VIDEO_V4L2
+ depends on COMMON_CLK
select VIDEO_OV7670
select VIDEOBUF2_VMALLOC
select VIDEOBUF2_DMA_CONTIG
@@ -14,6 +15,7 @@ config VIDEO_MMP_CAMERA
tristate "Marvell Armada 610 integrated camera controller support"
depends on I2C && VIDEO_V4L2
depends on ARCH_MMP || COMPILE_TEST
+ depends on COMMON_CLK
select VIDEO_OV7670
select I2C_GPIO
select VIDEOBUF2_VMALLOC
diff --git a/drivers/media/platform/marvell-ccic/cafe-driver.c
b/drivers/media/platform/marvell-ccic/cafe-driver.c
index 658294d319c0..0e712bb941ba 100644
--- a/drivers/media/platform/marvell-ccic/cafe-driver.c
+++ b/drivers/media/platform/marvell-ccic/cafe-driver.c
@@ -33,6 +33,7 @@
#include
#include
#include
+#include
#include "mcam-core.h"
@@ -533,11 +534,10 @@ static int cafe_pci_probe(struct pci_dev *pdev,
goto out_iounmap;
/*
-* Initialize the controller and leave it powered up. It will
-* stay that way until the sensor driver shows up.
+* Initialize the controller.
*/
cafe_ctlr_init(mcam);
- cafe_ctlr_power_up(mcam);
+
/*
* Set up I2C/SMBUS communications. We have to drop the mutex here
* because the sensor could attach in this call chain, leading to
@@ -555,6 +555,9 @@ static int cafe_pci_probe(struct pci_dev *pdev,
if (ret)
goto out_smbus_shutdown;
+ clkdev_create(mcam->mclk, "xclk", "%d-%04x",
+ i2c_adapter_id(cam->i2c_adapter), ov7670_info.addr);
+
if (i2c_new_device(cam->i2c_adapter, &ov7670_info)) {
cam->registered = 1;
return 0;
diff --git a/drivers/media/platform/marvell-ccic/mcam-core.c
b/drivers/media/platform/marvell-ccic/mcam-core.c
index 87812b7287f0..982fbac6472d 100644
--- a/drivers/media/platform/marvell-ccic/mcam-core.c
+++ b/drivers/media/platform/marvell-ccic/mcam-core.c
@@ -22,6 +22,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -303,9 +304,6 @@ static void mcam_enable_mipi(struct mcam_camera *mcam)
*/
mcam_reg_write(mcam, REG_CSI2_CTRL0,
CSI2_C0_MIPI_EN | CSI2_C0_ACT_LANE(mcam->lane));
- mcam_reg_write(mcam, REG_CLKCTRL,
- (mcam->mclk_src << 29) | mcam->mclk_div);
-
mcam->mipi_enabled = true;
}
}
@@ -846,11 +844,6 @@ static void mcam_ctlr_init(struct mcam_camera *cam)
* but it's good to be sure.
*/
mcam_reg_clear_bit(cam, REG_CTRL0, C0_ENABLE);
- /*
-* Clock the sensor appropriately. Controller clock should
-* be 48MHz, sensor "typical" value is half that.
-*/
- mcam_reg_write_mask(cam, REG_CLKCTRL, 2, CLK_DIV_MASK);
spin_unlock_irqrestore(&cam->dev_lock, flags);
}
@@ -898,14 +891,15 @@ static int mcam_ctlr_power_up(struct mcam_camera *cam)
int ret;
spin_lock_irqsave(&cam->dev_lock, flags);
- ret
[PATCH v3 13/14] [media] marvell-ccic: use async notifier to get the sensor
An instance of a sensor on DT-based MMP2 platform is always going to be
created asynchronously.
Let's move the manual device creation away from the core to the Cafe
driver (used on OLPC XO-1, not present in DT) and set up appropriate
async matches: I2C on Cafe, FWNODE on MMP (OLPC XO-1.75).
Signed-off-by: Lubomir Rintel
---
Changes since v2:
- Moved a typo fix hunk that was accidentally in the following patch
here, to unbreak build.
.../media/platform/marvell-ccic/cafe-driver.c | 49 --
.../media/platform/marvell-ccic/mcam-core.c | 157 --
.../media/platform/marvell-ccic/mcam-core.h | 5 +-
.../media/platform/marvell-ccic/mmp-driver.c | 27 +--
.../linux/platform_data/media/mmp-camera.h| 1 -
5 files changed, 162 insertions(+), 77 deletions(-)
diff --git a/drivers/media/platform/marvell-ccic/cafe-driver.c
b/drivers/media/platform/marvell-ccic/cafe-driver.c
index 2986cb4b88d0..658294d319c0 100644
--- a/drivers/media/platform/marvell-ccic/cafe-driver.c
+++ b/drivers/media/platform/marvell-ccic/cafe-driver.c
@@ -8,6 +8,7 @@
*
* Copyright 2006-11 One Laptop Per Child Association, Inc.
* Copyright 2006-11 Jonathan Corbet
+ * Copyright 2018 Lubomir Rintel
*
* Written by Jonathan Corbet, cor...@lwn.net.
*
@@ -27,6 +28,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -52,6 +54,7 @@ struct cafe_camera {
int registered; /* Fully initialized? */
struct mcam_camera mcam;
struct pci_dev *pdev;
+ struct i2c_adapter *i2c_adapter;
wait_queue_head_t smbus_wait; /* Waiting on i2c events */
};
@@ -351,15 +354,15 @@ static int cafe_smbus_setup(struct cafe_camera *cam)
return ret;
}
- cam->mcam.i2c_adapter = adap;
+ cam->i2c_adapter = adap;
cafe_smbus_enable_irq(cam);
return 0;
}
static void cafe_smbus_shutdown(struct cafe_camera *cam)
{
- i2c_del_adapter(cam->mcam.i2c_adapter);
- kfree(cam->mcam.i2c_adapter);
+ i2c_del_adapter(cam->i2c_adapter);
+ kfree(cam->i2c_adapter);
}
@@ -452,6 +455,29 @@ static irqreturn_t cafe_irq(int irq, void *data)
return IRQ_RETVAL(handled);
}
+/* --
*/
+
+static struct ov7670_config sensor_cfg = {
+ /*
+* Exclude QCIF mode, because it only captures a tiny portion
+* of the sensor FOV
+*/
+ .min_width = 320,
+ .min_height = 240,
+
+ /*
+* Set the clock speed for the XO 1; I don't believe this
+* driver has ever run anywhere else.
+*/
+ .clock_speed = 45,
+ .use_smbus = 1,
+};
+
+struct i2c_board_info ov7670_info = {
+ .type = "ov7670",
+ .addr = 0x42 >> 1,
+ .platform_data = &sensor_cfg,
+};
/* --
*/
/*
@@ -481,12 +507,6 @@ static int cafe_pci_probe(struct pci_dev *pdev,
mcam->plat_power_down = cafe_ctlr_power_down;
mcam->dev = &pdev->dev;
snprintf(mcam->bus_info, sizeof(mcam->bus_info), "PCI:%s",
pci_name(pdev));
- /*
-* Set the clock speed for the XO 1; I don't believe this
-* driver has ever run anywhere else.
-*/
- mcam->clock_speed = 45;
- mcam->use_smbus = 1;
/*
* Vmalloc mode for buffers is traditional with this driver.
* We *might* be able to run DMA_contig, especially on a system
@@ -527,12 +547,21 @@ static int cafe_pci_probe(struct pci_dev *pdev,
if (ret)
goto out_pdown;
+ mcam->asd.match_type = V4L2_ASYNC_MATCH_I2C;
+ mcam->asd.match.i2c.adapter_id = i2c_adapter_id(cam->i2c_adapter);
+ mcam->asd.match.i2c.address = ov7670_info.addr;
+
ret = mccic_register(mcam);
- if (ret == 0) {
+ if (ret)
+ goto out_smbus_shutdown;
+
+ if (i2c_new_device(cam->i2c_adapter, &ov7670_info)) {
cam->registered = 1;
return 0;
}
+ mccic_shutdown(mcam);
+out_smbus_shutdown:
cafe_smbus_shutdown(cam);
out_pdown:
cafe_ctlr_power_down(mcam);
diff --git a/drivers/media/platform/marvell-ccic/mcam-core.c
b/drivers/media/platform/marvell-ccic/mcam-core.c
index 0113b8d37d03..87812b7287f0 100644
--- a/drivers/media/platform/marvell-ccic/mcam-core.c
+++ b/drivers/media/platform/marvell-ccic/mcam-core.c
@@ -4,6 +4,7 @@
* so it needs platform-specific support outside of the core.
*
* Copyright 2011 Jonathan Corbet cor...@lwn.net
+ * Copyright 2018 Lubomir Rintel
*/
#include
#include
@@ -26,7 +27,6 @@
#include
#include
#include
-#include
#include
#include
#include
@@ -93,6 +93,9 @@ MODULE_PARM_DESC(buffer_mode,
#define sensor_call(
[PATCH v3 12/14] [media] marvell-ccic/mmp: add devicetree support
The platform data is actually not used anywhere (along with the CSI
support) and should be safe to remove.
Signed-off-by: Lubomir Rintel
Acked-by: Pavel Machek
---
Changes since v1:
- s/This are/These are/ in a comment
.../media/platform/marvell-ccic/mmp-driver.c | 36 ++-
1 file changed, 27 insertions(+), 9 deletions(-)
diff --git a/drivers/media/platform/marvell-ccic/mmp-driver.c
b/drivers/media/platform/marvell-ccic/mmp-driver.c
index 5c61317f56bc..05b9fa8c6a6f 100644
--- a/drivers/media/platform/marvell-ccic/mmp-driver.c
+++ b/drivers/media/platform/marvell-ccic/mmp-driver.c
@@ -19,6 +19,8 @@
#include
#include
#include
+#include
+#include
#include
#include
#include
@@ -196,6 +198,9 @@ static void mmpcam_calc_dphy(struct mcam_camera *mcam)
struct device *dev = &cam->pdev->dev;
unsigned long tx_clk_esc;
+ if (!pdata)
+ return;
+
/*
* If CSI2_DPHY3 is calculated dynamically,
* pdata->lane_clk should be already set
@@ -314,10 +319,6 @@ static int mmpcam_probe(struct platform_device *pdev)
struct mmp_camera_platform_data *pdata;
int ret;
- pdata = pdev->dev.platform_data;
- if (!pdata)
- return -ENODEV;
-
cam = devm_kzalloc(&pdev->dev, sizeof(*cam), GFP_KERNEL);
if (cam == NULL)
return -ENOMEM;
@@ -330,17 +331,29 @@ static int mmpcam_probe(struct platform_device *pdev)
mcam->calc_dphy = mmpcam_calc_dphy;
mcam->dev = &pdev->dev;
mcam->use_smbus = 0;
- mcam->mclk_src = pdata->mclk_src;
- mcam->mclk_div = pdata->mclk_div;
- mcam->bus_type = pdata->bus_type;
- mcam->dphy = pdata->dphy;
+ pdata = pdev->dev.platform_data;
+ if (pdata) {
+ mcam->mclk_src = pdata->mclk_src;
+ mcam->mclk_div = pdata->mclk_div;
+ mcam->bus_type = pdata->bus_type;
+ mcam->dphy = pdata->dphy;
+ mcam->lane = pdata->lane;
+ } else {
+ /*
+* These are values that used to be hardcoded in mcam-core and
+* work well on a OLPC XO 1.75 with a parallel bus sensor.
+* If it turns out other setups make sense, the values should
+* be obtained from the device tree.
+*/
+ mcam->mclk_src = 3;
+ mcam->mclk_div = 2;
+ }
if (mcam->bus_type == V4L2_MBUS_CSI2_DPHY) {
cam->mipi_clk = devm_clk_get(mcam->dev, "mipi");
if ((IS_ERR(cam->mipi_clk) && mcam->dphy[2] == 0))
return PTR_ERR(cam->mipi_clk);
}
mcam->mipi_enabled = false;
- mcam->lane = pdata->lane;
mcam->chip_id = MCAM_ARMADA610;
mcam->buffer_mode = B_DMA_sg;
strscpy(mcam->bus_info, "platform:mmp-camera", sizeof(mcam->bus_info));
@@ -475,6 +488,10 @@ static int mmpcam_resume(struct platform_device *pdev)
#endif
+static const struct of_device_id mmpcam_of_match[] = {
+ { .compatible = "marvell,mmp2-ccic", },
+ {},
+};
static struct platform_driver mmpcam_driver = {
.probe = mmpcam_probe,
@@ -485,6 +502,7 @@ static struct platform_driver mmpcam_driver = {
#endif
.driver = {
.name = "mmp-camera",
+ .of_match_table = of_match_ptr(mmpcam_of_match),
}
};
--
2.19.1
[PATCH v3 02/14] media: ov7670: split register setting from set_framerate() logic
This will allow us to restore the last set frame rate after the device
returns from a power off.
Signed-off-by: Lubomir Rintel
---
Changes since v2:
- This patch was added to the series
drivers/media/i2c/ov7670.c | 30 ++
1 file changed, 14 insertions(+), 16 deletions(-)
diff --git a/drivers/media/i2c/ov7670.c b/drivers/media/i2c/ov7670.c
index ee2302fbdeee..ead0c360df33 100644
--- a/drivers/media/i2c/ov7670.c
+++ b/drivers/media/i2c/ov7670.c
@@ -810,13 +810,24 @@ static void ov7675_get_framerate(struct v4l2_subdev *sd,
(4 * clkrc);
}
+static int ov7675_apply_framerate(struct v4l2_subdev *sd)
+{
+ struct ov7670_info *info = to_state(sd);
+ int ret;
+
+ ret = ov7670_write(sd, REG_CLKRC, info->clkrc);
+ if (ret < 0)
+ return ret;
+
+ return ov7670_write(sd, REG_DBLV, info->pll_bypass ? DBLV_BYPASS :
DBLV_X4);
+}
+
static int ov7675_set_framerate(struct v4l2_subdev *sd,
struct v4l2_fract *tpf)
{
struct ov7670_info *info = to_state(sd);
u32 clkrc;
int pll_factor;
- int ret;
/*
* The formula is fps = 5/4*pixclk for YUV/RGB and
@@ -825,19 +836,10 @@ static int ov7675_set_framerate(struct v4l2_subdev *sd,
* pixclk = clock_speed / (clkrc + 1) * PLLfactor
*
*/
- if (info->pll_bypass) {
- pll_factor = 1;
- ret = ov7670_write(sd, REG_DBLV, DBLV_BYPASS);
- } else {
- pll_factor = PLL_FACTOR;
- ret = ov7670_write(sd, REG_DBLV, DBLV_X4);
- }
- if (ret < 0)
- return ret;
-
if (tpf->numerator == 0 || tpf->denominator == 0) {
clkrc = 0;
} else {
+ pll_factor = info->pll_bypass ? 1 : PLL_FACTOR;
clkrc = (5 * pll_factor * info->clock_speed * tpf->numerator) /
(4 * tpf->denominator);
if (info->fmt->mbus_code == MEDIA_BUS_FMT_SBGGR8_1X8)
@@ -859,11 +861,7 @@ static int ov7675_set_framerate(struct v4l2_subdev *sd,
/* Recalculate frame rate */
ov7675_get_framerate(sd, tpf);
- ret = ov7670_write(sd, REG_CLKRC, info->clkrc);
- if (ret < 0)
- return ret;
-
- return ov7670_write(sd, REG_DBLV, DBLV_X4);
+ return ov7675_apply_framerate(sd);
}
static void ov7670_get_framerate_legacy(struct v4l2_subdev *sd,
--
2.19.1
[PATCH v3 0/14] media: make Marvell camera work on DT-based OLPC XO-1.75
Hi, this is the third spin of a patchset that modernizes the Marvel MMP2 CCIC driver. Notably, it ports it from the platform data (which seems unused as the board support code never made it) to devicetree. At the core of the rework is the move to asynchronous sensor discovery and clock management with the standard clock framework. There are also some straightforward fixes for the bitrotten parts. There's probably still room for improvement, but as it is, it seems to work well on OLPC XO-1.75 and doesn't break OLPC XO-1 (I've tested on both platforms). Changes since v2: - All documented in individual patches. Changes since v1: - "marvell-ccic: drop ctlr_reset()" patch was replaced with a straightforward revert of the commit that added ctlr_reset() along with an explanation in commit message. - Added collected Acks - Other changes are noted in individial patches Thanks, Lubo
[PATCH v2 0/11] media: make Marvell camera work on DT-based OLPC XO-1.75
Hello, this patch set somewhat modernizes the Marvel MMP2 CCIC driver. Notably, it ports it from the platform data (which seems unused as the board support code never made it) to devicetree. At the core of the rework is the move to asynchronous sensor discovery and clock management with the standard clock framework. There are also some straightforward fixes for bitrotten parts. There's probably still room for improvement, but as it is, it seems to work well on OLPC XO-1.75 and doesn't break OLPC XO-1 (I've tested on both platforms). Changes since v1: - "marvell-ccic: drop ctlr_reset()" patch was replaced with a straightforward revert of the commit that added ctlr_reset() along with an explanation in commit message. - Added collected Acks - Other changes are noted in individial patches Thanks, Lubo
[PATCH v2 05/11] [media] marvell-ccic: don't generate EOF on parallel bus
The commit 05fed81625bf ("[media] marvell-ccic: add MIPI support for
marvell-ccic driver") that claimed to add CSI2 turned on C0_EOF_VSYNC for
parallel bus without a very good explanation.
That broke camera on OLPC XO-1.75 which precisely uses a sensor on a
parallel bus. Revert that chunk.
Tested on an OLPC XO-1.75.
Fixes: 05fed81625bf755cc67c5864cdfd18b69ea828d1
Signed-off-by: Lubomir Rintel
---
drivers/media/platform/marvell-ccic/mcam-core.c | 6 --
1 file changed, 6 deletions(-)
diff --git a/drivers/media/platform/marvell-ccic/mcam-core.c
b/drivers/media/platform/marvell-ccic/mcam-core.c
index d97f39bde9bd..d24e5b7a3bc5 100644
--- a/drivers/media/platform/marvell-ccic/mcam-core.c
+++ b/drivers/media/platform/marvell-ccic/mcam-core.c
@@ -792,12 +792,6 @@ static void mcam_ctlr_image(struct mcam_camera *cam)
* Make sure it knows we want to use hsync/vsync.
*/
mcam_reg_write_mask(cam, REG_CTRL0, C0_SIF_HVSYNC, C0_SIFM_MASK);
- /*
-* This field controls the generation of EOF(DVP only)
-*/
- if (cam->bus_type != V4L2_MBUS_CSI2_DPHY)
- mcam_reg_set_bit(cam, REG_CTRL0,
- C0_EOF_VSYNC | C0_VEDGE_CTRL);
}
--
2.19.1
[PATCH v2 02/11] media: ov7670: control clock along with power
This provides more power saving when the sensor is off.
While at that, do the delay on power/clock enable even if the sensor driver
itself doesn't control the GPIOs. This is required for the OLPC XO-1
platform, that lacks the proper power/reset properties in its DT, but
needs the delay after the sensor is clocked up.
Signed-off-by: Lubomir Rintel
---
drivers/media/i2c/ov7670.c | 37 +
1 file changed, 21 insertions(+), 16 deletions(-)
diff --git a/drivers/media/i2c/ov7670.c b/drivers/media/i2c/ov7670.c
index d87f2362bf40..a3e72c62382c 100644
--- a/drivers/media/i2c/ov7670.c
+++ b/drivers/media/i2c/ov7670.c
@@ -241,6 +241,7 @@ struct ov7670_info {
struct v4l2_mbus_framefmt format;
struct ov7670_format_struct *fmt; /* Current format */
struct clk *clk;
+ int on;
struct gpio_desc *resetb_gpio;
struct gpio_desc *pwdn_gpio;
unsigned int mbus_config; /* Media bus configuration flags */
@@ -1610,15 +1611,26 @@ static int ov7670_s_power(struct v4l2_subdev *sd, int
on)
{
struct ov7670_info *info = to_state(sd);
+ if (info->on == on)
+ return 0;
+
+ if (on)
+ clk_prepare_enable(info->clk);
+ else
+ clk_disable_unprepare(info->clk);
+
if (info->pwdn_gpio)
gpiod_set_value(info->pwdn_gpio, !on);
if (on && info->resetb_gpio) {
gpiod_set_value(info->resetb_gpio, 1);
usleep_range(500, 1000);
gpiod_set_value(info->resetb_gpio, 0);
- usleep_range(3000, 5000);
}
+ if (on && (info->pwdn_gpio || info->resetb_gpio || info->clk))
+ usleep_range(3000, 5000);
+
+ info->on = on;
return 0;
}
@@ -1817,24 +1829,20 @@ static int ov7670_probe(struct i2c_client *client,
else
return ret;
}
- if (info->clk) {
- ret = clk_prepare_enable(info->clk);
- if (ret)
- return ret;
+ ret = ov7670_init_gpio(client, info);
+ if (ret)
+ return ret;
+
+ ov7670_s_power(sd, 1);
+ if (info->clk) {
info->clock_speed = clk_get_rate(info->clk) / 100;
if (info->clock_speed < 10 || info->clock_speed > 48) {
ret = -EINVAL;
- goto clk_disable;
+ goto power_off;
}
}
- ret = ov7670_init_gpio(client, info);
- if (ret)
- goto clk_disable;
-
- ov7670_s_power(sd, 1);
-
/* Make sure it's an ov7670 */
ret = ov7670_detect(sd);
if (ret) {
@@ -1913,6 +1921,7 @@ static int ov7670_probe(struct i2c_client *client,
if (ret < 0)
goto entity_cleanup;
+ ov7670_s_power(sd, 0);
return 0;
entity_cleanup:
@@ -1921,12 +1930,9 @@ static int ov7670_probe(struct i2c_client *client,
v4l2_ctrl_handler_free(&info->hdl);
power_off:
ov7670_s_power(sd, 0);
-clk_disable:
- clk_disable_unprepare(info->clk);
return ret;
}
-
static int ov7670_remove(struct i2c_client *client)
{
struct v4l2_subdev *sd = i2c_get_clientdata(client);
@@ -1934,7 +1940,6 @@ static int ov7670_remove(struct i2c_client *client)
v4l2_async_unregister_subdev(sd);
v4l2_ctrl_handler_free(&info->hdl);
- clk_disable_unprepare(info->clk);
media_entity_cleanup(&info->sd.entity);
ov7670_s_power(sd, 0);
return 0;
--
2.19.1
[PATCH v2 08/11] [media] marvell-ccic/mmp: enable clock before accessing registers
The access to REG_CLKCTRL or REG_CTRL1 without the clock enabled hangs the machine. Enable the clock first. Signed-off-by: Lubomir Rintel Acked-by: Pavel Machek --- drivers/media/platform/marvell-ccic/mmp-driver.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/drivers/media/platform/marvell-ccic/mmp-driver.c b/drivers/media/platform/marvell-ccic/mmp-driver.c index f2e43b23af18..05cba74c0d13 100644 --- a/drivers/media/platform/marvell-ccic/mmp-driver.c +++ b/drivers/media/platform/marvell-ccic/mmp-driver.c @@ -144,6 +144,7 @@ static int mmpcam_power_up(struct mcam_camera *mcam) * Turn on power and clocks to the controller. */ mmpcam_power_up_ctlr(cam); + mcam_clk_enable(mcam); /* * Provide power to the sensor. */ @@ -157,8 +158,6 @@ static int mmpcam_power_up(struct mcam_camera *mcam) gpio_set_value(pdata->sensor_reset_gpio, 1); /* reset is active low */ mdelay(5); - mcam_clk_enable(mcam); - return 0; } -- 2.19.1
[PATCH v2 06/11] Revert "[media] marvell-ccic: reset ccic phy when stop streaming for stability"
This accesses the clock registers directly and thus is going to stay in the
way of making the driver devicetree friendly.
No boards seems to actually use this. If it's somehow actually needed it
needs to be done differently.
This reverts commit 7c269f454e7a51b151d94f99344120efa1cd0acb.
---
.../media/platform/marvell-ccic/mcam-core.c | 6 -
.../media/platform/marvell-ccic/mcam-core.h | 2 --
.../media/platform/marvell-ccic/mmp-driver.c | 25 ---
3 files changed, 33 deletions(-)
diff --git a/drivers/media/platform/marvell-ccic/mcam-core.c
b/drivers/media/platform/marvell-ccic/mcam-core.c
index d24e5b7a3bc5..1b879035948c 100644
--- a/drivers/media/platform/marvell-ccic/mcam-core.c
+++ b/drivers/media/platform/marvell-ccic/mcam-core.c
@@ -1154,12 +1154,6 @@ static void mcam_vb_stop_streaming(struct vb2_queue *vq)
if (cam->state != S_STREAMING)
return;
mcam_ctlr_stop_dma(cam);
- /*
-* Reset the CCIC PHY after stopping streaming,
-* otherwise, the CCIC may be unstable.
-*/
- if (cam->ctlr_reset)
- cam->ctlr_reset(cam);
/*
* VB2 reclaims the buffers, so we need to forget
* about them.
diff --git a/drivers/media/platform/marvell-ccic/mcam-core.h
b/drivers/media/platform/marvell-ccic/mcam-core.h
index ad8955f9f0a1..a3a097a45e78 100644
--- a/drivers/media/platform/marvell-ccic/mcam-core.h
+++ b/drivers/media/platform/marvell-ccic/mcam-core.h
@@ -116,7 +116,6 @@ struct mcam_camera {
int mclk_src; /* which clock source the mclk derives from */
int mclk_div; /* Clock Divider Value for MCLK */
- int ccic_id;
enum v4l2_mbus_type bus_type;
/* MIPI support */
/* The dphy config value, allocated in board file
@@ -137,7 +136,6 @@ struct mcam_camera {
int (*plat_power_up) (struct mcam_camera *cam);
void (*plat_power_down) (struct mcam_camera *cam);
void (*calc_dphy) (struct mcam_camera *cam);
- void (*ctlr_reset) (struct mcam_camera *cam);
/*
* Everything below here is private to the mcam core and
diff --git a/drivers/media/platform/marvell-ccic/mmp-driver.c
b/drivers/media/platform/marvell-ccic/mmp-driver.c
index 70a2833db0d1..dbfc597b955d 100644
--- a/drivers/media/platform/marvell-ccic/mmp-driver.c
+++ b/drivers/media/platform/marvell-ccic/mmp-driver.c
@@ -105,7 +105,6 @@ static struct mmp_camera *mmpcam_find_device(struct
platform_device *pdev)
#define CPU_SUBSYS_PMU_BASE0xd4282800
#define REG_CCIC_DCGCR 0x28/* CCIC dyn clock gate ctrl reg */
#define REG_CCIC_CRCR 0x50/* CCIC clk reset ctrl reg */
-#define REG_CCIC2_CRCR 0xf4/* CCIC2 clk reset ctrl reg */
static void mcam_clk_enable(struct mcam_camera *mcam)
{
@@ -183,28 +182,6 @@ static void mmpcam_power_down(struct mcam_camera *mcam)
mcam_clk_disable(mcam);
}
-static void mcam_ctlr_reset(struct mcam_camera *mcam)
-{
- unsigned long val;
- struct mmp_camera *cam = mcam_to_cam(mcam);
-
- if (mcam->ccic_id) {
- /*
-* Using CCIC2
-*/
- val = ioread32(cam->power_regs + REG_CCIC2_CRCR);
- iowrite32(val & ~0x2, cam->power_regs + REG_CCIC2_CRCR);
- iowrite32(val | 0x2, cam->power_regs + REG_CCIC2_CRCR);
- } else {
- /*
-* Using CCIC1
-*/
- val = ioread32(cam->power_regs + REG_CCIC_CRCR);
- iowrite32(val & ~0x2, cam->power_regs + REG_CCIC_CRCR);
- iowrite32(val | 0x2, cam->power_regs + REG_CCIC_CRCR);
- }
-}
-
/*
* calc the dphy register values
* There are three dphy registers being used.
@@ -352,11 +329,9 @@ static int mmpcam_probe(struct platform_device *pdev)
mcam = &cam->mcam;
mcam->plat_power_up = mmpcam_power_up;
mcam->plat_power_down = mmpcam_power_down;
- mcam->ctlr_reset = mcam_ctlr_reset;
mcam->calc_dphy = mmpcam_calc_dphy;
mcam->dev = &pdev->dev;
mcam->use_smbus = 0;
- mcam->ccic_id = pdev->id;
mcam->mclk_min = pdata->mclk_min;
mcam->mclk_src = pdata->mclk_src;
mcam->mclk_div = pdata->mclk_div;
--
2.19.1
[PATCH v2 11/11] [media] marvell-ccic: provide a clock for the sensor
The sensor needs the MCLK clock running when it's being probed. On
platforms where the sensor is instantiated from a DT (MMP2) it is going
to happen asynchronously.
Therefore, the current modus operandi, where the bridge driver fiddles
with the sensor power and clock itself is not going to fly. As the comments
wisely note, this doesn't even belong there.
Luckily, the ov7670 driver is already able to control its power and
reset lines, we can just drop the MMP platform glue altogether.
It also requests the clock via the standard clock subsystem. Good -- let's
set up a clock instance so that the sensor can ask us to enable the clock.
Note that this is pretty dumb at the moment: the clock is hardwired to a
particular frequency and parent. It was always the case.
Signed-off-by: Lubomir Rintel
---
Changes since v1:
- [kbuild/ia64] depend on COMMON_CLK.
- [smatch] fix bad return in mcam_v4l_open() leading to lock not getting
released on error.
drivers/media/platform/marvell-ccic/Kconfig | 2 +
.../media/platform/marvell-ccic/cafe-driver.c | 11 +-
.../media/platform/marvell-ccic/mcam-core.c | 156 +++---
.../media/platform/marvell-ccic/mcam-core.h | 3 +
.../media/platform/marvell-ccic/mmp-driver.c | 152 ++---
.../linux/platform_data/media/mmp-camera.h| 2 -
6 files changed, 162 insertions(+), 164 deletions(-)
diff --git a/drivers/media/platform/marvell-ccic/Kconfig
b/drivers/media/platform/marvell-ccic/Kconfig
index cf12e077203a..3e12eb25740a 100644
--- a/drivers/media/platform/marvell-ccic/Kconfig
+++ b/drivers/media/platform/marvell-ccic/Kconfig
@@ -1,6 +1,7 @@
config VIDEO_CAFE_CCIC
tristate "Marvell 88ALP01 (Cafe) CMOS Camera Controller support"
depends on PCI && I2C && VIDEO_V4L2
+ depends on COMMON_CLK
select VIDEO_OV7670
select VIDEOBUF2_VMALLOC
select VIDEOBUF2_DMA_CONTIG
@@ -14,6 +15,7 @@ config VIDEO_MMP_CAMERA
tristate "Marvell Armada 610 integrated camera controller support"
depends on I2C && VIDEO_V4L2
depends on ARCH_MMP || COMPILE_TEST
+ depends on COMMON_CLK
select VIDEO_OV7670
select I2C_GPIO
select VIDEOBUF2_VMALLOC
diff --git a/drivers/media/platform/marvell-ccic/cafe-driver.c
b/drivers/media/platform/marvell-ccic/cafe-driver.c
index 0164afc405d1..0e712bb941ba 100644
--- a/drivers/media/platform/marvell-ccic/cafe-driver.c
+++ b/drivers/media/platform/marvell-ccic/cafe-driver.c
@@ -33,6 +33,7 @@
#include
#include
#include
+#include
#include "mcam-core.h"
@@ -533,11 +534,10 @@ static int cafe_pci_probe(struct pci_dev *pdev,
goto out_iounmap;
/*
-* Initialize the controller and leave it powered up. It will
-* stay that way until the sensor driver shows up.
+* Initialize the controller.
*/
cafe_ctlr_init(mcam);
- cafe_ctlr_power_up(mcam);
+
/*
* Set up I2C/SMBUS communications. We have to drop the mutex here
* because the sensor could attach in this call chain, leading to
@@ -555,12 +555,15 @@ static int cafe_pci_probe(struct pci_dev *pdev,
if (ret)
goto out_smbus_shutdown;
+ clkdev_create(mcam->mclk, "xclk", "%d-%04x",
+ i2c_adapter_id(cam->i2c_adapter), ov7670_info.addr);
+
if (i2c_new_device(cam->i2c_adapter, &ov7670_info)) {
cam->registered = 1;
return 0;
}
- cafe_shutdown(cam);
+ mccic_shutdown(mcam);
out_smbus_shutdown:
cafe_smbus_shutdown(cam);
out_pdown:
diff --git a/drivers/media/platform/marvell-ccic/mcam-core.c
b/drivers/media/platform/marvell-ccic/mcam-core.c
index 87812b7287f0..982fbac6472d 100644
--- a/drivers/media/platform/marvell-ccic/mcam-core.c
+++ b/drivers/media/platform/marvell-ccic/mcam-core.c
@@ -22,6 +22,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -303,9 +304,6 @@ static void mcam_enable_mipi(struct mcam_camera *mcam)
*/
mcam_reg_write(mcam, REG_CSI2_CTRL0,
CSI2_C0_MIPI_EN | CSI2_C0_ACT_LANE(mcam->lane));
- mcam_reg_write(mcam, REG_CLKCTRL,
- (mcam->mclk_src << 29) | mcam->mclk_div);
-
mcam->mipi_enabled = true;
}
}
@@ -846,11 +844,6 @@ static void mcam_ctlr_init(struct mcam_camera *cam)
* but it's good to be sure.
*/
mcam_reg_clear_bit(cam, REG_CTRL0, C0_ENABLE);
- /*
-* Clock the sensor appropriately. Controller clock should
-* be 48MHz, sensor "typical" value is half that.
-*/
- mcam_reg_write_mask(cam, REG_CLKCTRL, 2, CLK_DIV_MASK);
spin_unlock_irqrestore(&cam->dev_lock, flags);
}
@@ -898,14 +891,15 @@ static int
[PATCH v2 10/11] [media] marvell-ccic: use async notifier to get the sensor
An instance of a sensor on DT-based MMP2 platform is always going to be
created asynchronously.
Let's move the manual device creation away from the core to the Cafe
driver (used on OLPC XO-1, not present in DT) and set up appropriate
async matches: I2C on Cafe, FWNODE on MMP (OLPC XO-1.75).
Signed-off-by: Lubomir Rintel
---
.../media/platform/marvell-ccic/cafe-driver.c | 49 --
.../media/platform/marvell-ccic/mcam-core.c | 157 --
.../media/platform/marvell-ccic/mcam-core.h | 5 +-
.../media/platform/marvell-ccic/mmp-driver.c | 27 +--
.../linux/platform_data/media/mmp-camera.h| 1 -
5 files changed, 162 insertions(+), 77 deletions(-)
diff --git a/drivers/media/platform/marvell-ccic/cafe-driver.c
b/drivers/media/platform/marvell-ccic/cafe-driver.c
index 2986cb4b88d0..0164afc405d1 100644
--- a/drivers/media/platform/marvell-ccic/cafe-driver.c
+++ b/drivers/media/platform/marvell-ccic/cafe-driver.c
@@ -8,6 +8,7 @@
*
* Copyright 2006-11 One Laptop Per Child Association, Inc.
* Copyright 2006-11 Jonathan Corbet
+ * Copyright 2018 Lubomir Rintel
*
* Written by Jonathan Corbet, cor...@lwn.net.
*
@@ -27,6 +28,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -52,6 +54,7 @@ struct cafe_camera {
int registered; /* Fully initialized? */
struct mcam_camera mcam;
struct pci_dev *pdev;
+ struct i2c_adapter *i2c_adapter;
wait_queue_head_t smbus_wait; /* Waiting on i2c events */
};
@@ -351,15 +354,15 @@ static int cafe_smbus_setup(struct cafe_camera *cam)
return ret;
}
- cam->mcam.i2c_adapter = adap;
+ cam->i2c_adapter = adap;
cafe_smbus_enable_irq(cam);
return 0;
}
static void cafe_smbus_shutdown(struct cafe_camera *cam)
{
- i2c_del_adapter(cam->mcam.i2c_adapter);
- kfree(cam->mcam.i2c_adapter);
+ i2c_del_adapter(cam->i2c_adapter);
+ kfree(cam->i2c_adapter);
}
@@ -452,6 +455,29 @@ static irqreturn_t cafe_irq(int irq, void *data)
return IRQ_RETVAL(handled);
}
+/* --
*/
+
+static struct ov7670_config sensor_cfg = {
+ /*
+* Exclude QCIF mode, because it only captures a tiny portion
+* of the sensor FOV
+*/
+ .min_width = 320,
+ .min_height = 240,
+
+ /*
+* Set the clock speed for the XO 1; I don't believe this
+* driver has ever run anywhere else.
+*/
+ .clock_speed = 45,
+ .use_smbus = 1,
+};
+
+struct i2c_board_info ov7670_info = {
+ .type = "ov7670",
+ .addr = 0x42 >> 1,
+ .platform_data = &sensor_cfg,
+};
/* --
*/
/*
@@ -481,12 +507,6 @@ static int cafe_pci_probe(struct pci_dev *pdev,
mcam->plat_power_down = cafe_ctlr_power_down;
mcam->dev = &pdev->dev;
snprintf(mcam->bus_info, sizeof(mcam->bus_info), "PCI:%s",
pci_name(pdev));
- /*
-* Set the clock speed for the XO 1; I don't believe this
-* driver has ever run anywhere else.
-*/
- mcam->clock_speed = 45;
- mcam->use_smbus = 1;
/*
* Vmalloc mode for buffers is traditional with this driver.
* We *might* be able to run DMA_contig, especially on a system
@@ -527,12 +547,21 @@ static int cafe_pci_probe(struct pci_dev *pdev,
if (ret)
goto out_pdown;
+ mcam->asd.match_type = V4L2_ASYNC_MATCH_I2C;
+ mcam->asd.match.i2c.adapter_id = i2c_adapter_id(cam->i2c_adapter);
+ mcam->asd.match.i2c.address = ov7670_info.addr;
+
ret = mccic_register(mcam);
- if (ret == 0) {
+ if (ret)
+ goto out_smbus_shutdown;
+
+ if (i2c_new_device(cam->i2c_adapter, &ov7670_info)) {
cam->registered = 1;
return 0;
}
+ cafe_shutdown(cam);
+out_smbus_shutdown:
cafe_smbus_shutdown(cam);
out_pdown:
cafe_ctlr_power_down(mcam);
diff --git a/drivers/media/platform/marvell-ccic/mcam-core.c
b/drivers/media/platform/marvell-ccic/mcam-core.c
index 0113b8d37d03..87812b7287f0 100644
--- a/drivers/media/platform/marvell-ccic/mcam-core.c
+++ b/drivers/media/platform/marvell-ccic/mcam-core.c
@@ -4,6 +4,7 @@
* so it needs platform-specific support outside of the core.
*
* Copyright 2011 Jonathan Corbet cor...@lwn.net
+ * Copyright 2018 Lubomir Rintel
*/
#include
#include
@@ -26,7 +27,6 @@
#include
#include
#include
-#include
#include
#include
#include
@@ -93,6 +93,9 @@ MODULE_PARM_DESC(buffer_mode,
#define sensor_call(cam, o, f, args...) \
v4l2_subdev_call(cam->sensor, o, f, ##args)
+#define notifier_to_mcam(notifier) \
+ container_o
[PATCH v2 04/11] [media] marvell-ccic: fix DMA s/g desc number calculation
The commit d790b7eda953 ("[media] vb2-dma-sg: move dma_(un)map_sg here")
left dma_desc_nent unset. It previously contained the number of DMA
descriptors as returned from dma_map_sg().
We can now (since the commit referred to above) obtain the same value from
the sg_table and drop dma_desc_nent altogether.
Tested on OLPC XO-1.75 machine. Doesn't affect the OLPC XO-1's Cafe
driver, since that one doesn't do DMA.
Fixes: d790b7eda953df474f470169ebdf111c02fa7a2d
Signed-off-by: Lubomir Rintel
---
drivers/media/platform/marvell-ccic/mcam-core.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/media/platform/marvell-ccic/mcam-core.c
b/drivers/media/platform/marvell-ccic/mcam-core.c
index f1b301810260..d97f39bde9bd 100644
--- a/drivers/media/platform/marvell-ccic/mcam-core.c
+++ b/drivers/media/platform/marvell-ccic/mcam-core.c
@@ -200,7 +200,6 @@ struct mcam_vb_buffer {
struct list_head queue;
struct mcam_dma_desc *dma_desc; /* Descriptor virtual address */
dma_addr_t dma_desc_pa; /* Descriptor physical address */
- int dma_desc_nent; /* Number of mapped descriptors */
};
static inline struct mcam_vb_buffer *vb_to_mvb(struct vb2_v4l2_buffer *vb)
@@ -608,9 +607,11 @@ static void mcam_dma_contig_done(struct mcam_camera *cam,
int frame)
static void mcam_sg_next_buffer(struct mcam_camera *cam)
{
struct mcam_vb_buffer *buf;
+ struct sg_table *sg_table;
buf = list_first_entry(&cam->buffers, struct mcam_vb_buffer, queue);
list_del_init(&buf->queue);
+ sg_table = vb2_dma_sg_plane_desc(&buf->vb_buf.vb2_buf, 0);
/*
* Very Bad Not Good Things happen if you don't clear
* C1_DESC_ENA before making any descriptor changes.
@@ -618,7 +619,7 @@ static void mcam_sg_next_buffer(struct mcam_camera *cam)
mcam_reg_clear_bit(cam, REG_CTRL1, C1_DESC_ENA);
mcam_reg_write(cam, REG_DMA_DESC_Y, buf->dma_desc_pa);
mcam_reg_write(cam, REG_DESC_LEN_Y,
- buf->dma_desc_nent*sizeof(struct mcam_dma_desc));
+ sg_table->nents*sizeof(struct mcam_dma_desc));
mcam_reg_write(cam, REG_DESC_LEN_U, 0);
mcam_reg_write(cam, REG_DESC_LEN_V, 0);
mcam_reg_set_bit(cam, REG_CTRL1, C1_DESC_ENA);
--
2.19.1
[PATCH v2 09/11] [media] marvell-ccic/mmp: add devicetree support
The platform data is actually not used anywhere (along with the CSI
support) and should be safe to remove.
Signed-off-by: Lubomir Rintel
Acked-by: Pavel Machek
---
Changes since v1:
- s/This are/These are/ in a comment
.../media/platform/marvell-ccic/mmp-driver.c | 36 ++-
1 file changed, 27 insertions(+), 9 deletions(-)
diff --git a/drivers/media/platform/marvell-ccic/mmp-driver.c
b/drivers/media/platform/marvell-ccic/mmp-driver.c
index 05cba74c0d13..0f65dbd1de66 100644
--- a/drivers/media/platform/marvell-ccic/mmp-driver.c
+++ b/drivers/media/platform/marvell-ccic/mmp-driver.c
@@ -19,6 +19,8 @@
#include
#include
#include
+#include
+#include
#include
#include
#include
@@ -196,6 +198,9 @@ static void mmpcam_calc_dphy(struct mcam_camera *mcam)
struct device *dev = &cam->pdev->dev;
unsigned long tx_clk_esc;
+ if (!pdata)
+ return;
+
/*
* If CSI2_DPHY3 is calculated dynamically,
* pdata->lane_clk should be already set
@@ -314,10 +319,6 @@ static int mmpcam_probe(struct platform_device *pdev)
struct mmp_camera_platform_data *pdata;
int ret;
- pdata = pdev->dev.platform_data;
- if (!pdata)
- return -ENODEV;
-
cam = devm_kzalloc(&pdev->dev, sizeof(*cam), GFP_KERNEL);
if (cam == NULL)
return -ENOMEM;
@@ -330,17 +331,29 @@ static int mmpcam_probe(struct platform_device *pdev)
mcam->calc_dphy = mmpcam_calc_dphy;
mcam->dev = &pdev->dev;
mcam->use_smbus = 0;
- mcam->mclk_src = pdata->mclk_src;
- mcam->mclk_div = pdata->mclk_div;
- mcam->bus_type = pdata->bus_type;
- mcam->dphy = pdata->dphy;
+ pdata = pdev->dev.platform_data;
+ if (pdata) {
+ mcam->mclk_src = pdata->mclk_src;
+ mcam->mclk_div = pdata->mclk_div;
+ mcam->bus_type = pdata->bus_type;
+ mcam->dphy = pdata->dphy;
+ mcam->lane = pdata->lane;
+ } else {
+ /*
+* These are values that used to be hardcoded in mcam-core and
+* work well on a OLPC XO 1.75 with a parallel bus sensor.
+* If it turns out other setups make sense, the values should
+* be obtained from the device tree.
+*/
+ mcam->mclk_src = 3;
+ mcam->mclk_div = 2;
+ }
if (mcam->bus_type == V4L2_MBUS_CSI2_DPHY) {
cam->mipi_clk = devm_clk_get(mcam->dev, "mipi");
if ((IS_ERR(cam->mipi_clk) && mcam->dphy[2] == 0))
return PTR_ERR(cam->mipi_clk);
}
mcam->mipi_enabled = false;
- mcam->lane = pdata->lane;
mcam->chip_id = MCAM_ARMADA610;
mcam->buffer_mode = B_DMA_sg;
strscpy(mcam->bus_info, "platform:mmp-camera", sizeof(mcam->bus_info));
@@ -475,6 +488,10 @@ static int mmpcam_resume(struct platform_device *pdev)
#endif
+static const struct of_device_id mmpcam_of_match[] = {
+ { .compatible = "marvell,mmp2-ccic", },
+ {},
+};
static struct platform_driver mmpcam_driver = {
.probe = mmpcam_probe,
@@ -485,6 +502,7 @@ static struct platform_driver mmpcam_driver = {
#endif
.driver = {
.name = "mmp-camera",
+ .of_match_table = of_match_ptr(mmpcam_of_match),
}
};
--
2.19.1
[PATCH v2 07/11] [media] marvell-ccic: drop unused stuff
Remove structure members and headers that are not actually used. Saves
us from some noise in subsequent cleanup commits.
Signed-off-by: Lubomir Rintel
Acked-by: Pavel Machek
---
drivers/media/platform/marvell-ccic/mcam-core.c | 1 -
drivers/media/platform/marvell-ccic/mcam-core.h | 2 --
drivers/media/platform/marvell-ccic/mmp-driver.c | 2 --
include/linux/platform_data/media/mmp-camera.h | 1 -
4 files changed, 6 deletions(-)
diff --git a/drivers/media/platform/marvell-ccic/mcam-core.c
b/drivers/media/platform/marvell-ccic/mcam-core.c
index 1b879035948c..0113b8d37d03 100644
--- a/drivers/media/platform/marvell-ccic/mcam-core.c
+++ b/drivers/media/platform/marvell-ccic/mcam-core.c
@@ -1776,7 +1776,6 @@ int mccic_register(struct mcam_camera *cam)
*/
sensor_cfg.clock_speed = cam->clock_speed;
sensor_cfg.use_smbus = cam->use_smbus;
- cam->sensor_addr = ov7670_info.addr;
cam->sensor = v4l2_i2c_new_subdev_board(&cam->v4l2_dev,
cam->i2c_adapter, &ov7670_info, NULL);
if (cam->sensor == NULL) {
diff --git a/drivers/media/platform/marvell-ccic/mcam-core.h
b/drivers/media/platform/marvell-ccic/mcam-core.h
index a3a097a45e78..b828b1bb59d3 100644
--- a/drivers/media/platform/marvell-ccic/mcam-core.h
+++ b/drivers/media/platform/marvell-ccic/mcam-core.h
@@ -112,7 +112,6 @@ struct mcam_camera {
short int use_smbus;/* SMBUS or straight I2c? */
enum mcam_buffer_mode buffer_mode;
- int mclk_min; /* The minimal value of mclk */
int mclk_src; /* which clock source the mclk derives from */
int mclk_div; /* Clock Divider Value for MCLK */
@@ -152,7 +151,6 @@ struct mcam_camera {
*/
struct video_device vdev;
struct v4l2_subdev *sensor;
- unsigned short sensor_addr;
/* Videobuf2 stuff */
struct vb2_queue vb_queue;
diff --git a/drivers/media/platform/marvell-ccic/mmp-driver.c
b/drivers/media/platform/marvell-ccic/mmp-driver.c
index dbfc597b955d..f2e43b23af18 100644
--- a/drivers/media/platform/marvell-ccic/mmp-driver.c
+++ b/drivers/media/platform/marvell-ccic/mmp-driver.c
@@ -12,7 +12,6 @@
#include
#include
#include
-#include
#include
#include
#include
@@ -332,7 +331,6 @@ static int mmpcam_probe(struct platform_device *pdev)
mcam->calc_dphy = mmpcam_calc_dphy;
mcam->dev = &pdev->dev;
mcam->use_smbus = 0;
- mcam->mclk_min = pdata->mclk_min;
mcam->mclk_src = pdata->mclk_src;
mcam->mclk_div = pdata->mclk_div;
mcam->bus_type = pdata->bus_type;
diff --git a/include/linux/platform_data/media/mmp-camera.h
b/include/linux/platform_data/media/mmp-camera.h
index d2d3a443eedf..4c3a80a45883 100644
--- a/include/linux/platform_data/media/mmp-camera.h
+++ b/include/linux/platform_data/media/mmp-camera.h
@@ -16,7 +16,6 @@ struct mmp_camera_platform_data {
int sensor_power_gpio;
int sensor_reset_gpio;
enum v4l2_mbus_type bus_type;
- int mclk_min; /* The minimal value of MCLK */
int mclk_src; /* which clock source the MCLK derives from */
int mclk_div; /* Clock Divider Value for MCLK */
/*
--
2.19.1
[PATCH v2 03/11] media: dt-bindings: marvell,mmp2-ccic: Add Marvell MMP2 camera
Add Marvell MMP2 camera host interface.
Signed-off-by: Lubomir Rintel
---
.../bindings/media/marvell,mmp2-ccic.txt | 30 +++
1 file changed, 30 insertions(+)
create mode 100644
Documentation/devicetree/bindings/media/marvell,mmp2-ccic.txt
diff --git a/Documentation/devicetree/bindings/media/marvell,mmp2-ccic.txt
b/Documentation/devicetree/bindings/media/marvell,mmp2-ccic.txt
new file mode 100644
index ..a9c536e58dda
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/marvell,mmp2-ccic.txt
@@ -0,0 +1,30 @@
+Marvell MMP2 camera host interface
+
+Required properties:
+ - compatible: Should be "marvell,mmp2-ccic"
+ - reg: register base and size
+ - interrupts: the interrupt number
+ - any required generic properties defined in video-interfaces.txt
+
+Optional properties:
+ - clocks: input clock (see clock-bindings.txt)
+ - clock-output-names: should contain the name of the clock driving the
+ sensor master clock MCLK
+
+Example:
+
+ camera0: camera@d420a000 {
+ compatible = "marvell,mmp2-ccic";
+ reg = <0xd420a000 0x800>;
+ interrupts = <42>;
+ clocks = <&soc_clocks MMP2_CLK_CCIC0>;
+ clock-names = "CCICAXICLK";
+ #clock-cells = <0>;
+ clock-output-names = "mclk";
+
+ port {
+ camera0_0: endpoint {
+ remote-endpoint = <&ov7670_0>;
+ };
+ };
+ };
--
2.19.1
[PATCH v2 01/11] media: ov7670: hook s_power onto v4l2 core
The commit 71862f63f351 ("media: ov7670: Add the ov7670_s_power function")
added a s_power function. For some reason it didn't register it with v4l2,
only uses it internally. Fix this now.
Signed-off-by: Lubomir Rintel
---
drivers/media/i2c/ov7670.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/media/i2c/ov7670.c b/drivers/media/i2c/ov7670.c
index bc68a3a5b4ec..d87f2362bf40 100644
--- a/drivers/media/i2c/ov7670.c
+++ b/drivers/media/i2c/ov7670.c
@@ -1651,6 +1651,7 @@ static int ov7670_open(struct v4l2_subdev *sd, struct
v4l2_subdev_fh *fh)
static const struct v4l2_subdev_core_ops ov7670_core_ops = {
.reset = ov7670_reset,
.init = ov7670_init,
+ .s_power = ov7670_s_power,
#ifdef CONFIG_VIDEO_ADV_DEBUG
.g_register = ov7670_g_register,
.s_register = ov7670_s_register,
--
2.19.1
Re: [PATCH] [media] ov7670: make "xclk" clock optional
On Mon, 2018-11-05 at 18:15 +0100, jacopo mondi wrote:
> Hi Lubo,
>
> On Mon, Nov 05, 2018 at 04:22:15PM +0100, Lubomir Rintel wrote:
> > On Mon, 2018-11-05 at 15:22 +0100, jacopo mondi wrote:
> > > Hi Lubo,
> > >
> > > On Mon, Nov 05, 2018 at 02:12:18PM +0100, Lubomir Rintel wrote:
> > > > Hello,
> > > >
> > > > On Mon, 2018-11-05 at 11:58 +0100, jacopo mondi wrote:
> > > > > Hi Lubomir,
> > > > >+Sakari in Cc
> > > > >
> > > > > I just noticed this, and the patch is now in v4.20, but let me comment
> > > > > anyway on this.
> > > > >
> > > > > On Thu, Oct 04, 2018 at 11:29:03PM +0200, Lubomir Rintel wrote:
> > > > > > When the "xclk" clock was added, it was made mandatory. This broke
> > > > > > the
> > > > > > driver on an OLPC plaform which doesn't know such clock. Make it
> > > > > > optional.
> > > > > >
> > > > >
> > > > > I don't think this is correct. The sensor needs a clock to work.
> > > > >
> > > > > With this patch clock_speed which is used to calculate
> > > > > the framerate is defaulted to 30MHz, crippling all the calculations if
> > > > > that default value doesn't match what is actually installed on the
> > > > > board.
> > > >
> > > > How come? I kept this:
> > > >
> > > > + info->clock_speed = clk_get_rate(info->clk) / 100;
> > >
> > > Yes, but only if
> > > if (info->clk) { }
> > >
> > > if (!info->clk) the 'clock_speed' variable is defaulted to 30 at the
> > > beginning of the probe routine. Am I missing something obvious here?
> >
> > Maybe. Or I am.
> >
> > I thought you care about the situation where you *have* the clk, and
> > thus you shouldn't be caring about the defaults?
> >
>
> I care about the fact that with this version, the clock speed might be
> default to a totally random value making the driver malfunctioning. My
> specific use case doesn't matter.
>
> > > > > If this patch breaks the OLPC, then might it be the DTS for said
> > > > > device needs to be fixed instead of working around the issue here?
> > > >
> > > > No. Device tree is an ABI, and you can't just add mandatory properties.
> > > >
> > >
> > > Well, as I read the ov7670 bindings documentation:
> > >
> > > Required Properties:
> > > - compatible: should be "ovti,ov7670"
> > > - clocks: reference to the xclk input clock.
> > > - clock-names: should be "xclk".
> > >
> > > It was mandatory already since the bindings have been first created:
> > > bba582894a ("[media] ov7670: document device tree bindings")
> > >
> > > And yes, bindings establishes an ABI we have not to break or make
> > > incompatible with DTs created for an older version of the same binding,
> > > but the DTs itself is free to change and we need to do so to update
> > > it when required (to fix bugs, add new components, enable/disable them
> > > etc).
> >
> > Ah, right, you're correct. No DTS ABI breakage there. I guess it would
> > be fine to revert my patch if we provide the xclk on the OLPC instead.
> >
>
> Thanks I feel that would be the right thing to do.
>
> > > > There's no DTS for OLPC XO-1 either; it's an OpenFirmware machine.
> > > >
> > >
> > > I thought OLPC was an ARM machine, that's why I mentioned DTS. Sorry
> > > about this.
> >
> > Well, you're sort of right here. The XO-1.75 generation is ARM, the XO-
> > 1 is x86. They both use devicetree provided by the firmware. However,
> > they predate FDT (or the definitions of bindings that are used here for
> > tht matter) and the trees are unfortunately quite incomplete. The
> > sensor is not there.
>
> Ouch. I see...
>
> > > A quick read of the wikipedia page for "OpenFirmware" gives me back
> > > that it a standardized firmware interface:
> > > "Open Firmware allows the system to load platform-independent drivers
> > > directly from the PCI card, improving compatibility".
> > >
> > > I know nothing on this, and that's not the point, so I'll better stop
> > > here a
Re: [PATCH] [media] ov7670: make "xclk" clock optional
On Mon, 2018-11-05 at 15:22 +0100, jacopo mondi wrote:
> Hi Lubo,
>
> On Mon, Nov 05, 2018 at 02:12:18PM +0100, Lubomir Rintel wrote:
> > Hello,
> >
> > On Mon, 2018-11-05 at 11:58 +0100, jacopo mondi wrote:
> > > Hi Lubomir,
> > >+Sakari in Cc
> > >
> > > I just noticed this, and the patch is now in v4.20, but let me comment
> > > anyway on this.
> > >
> > > On Thu, Oct 04, 2018 at 11:29:03PM +0200, Lubomir Rintel wrote:
> > > > When the "xclk" clock was added, it was made mandatory. This broke the
> > > > driver on an OLPC plaform which doesn't know such clock. Make it
> > > > optional.
> > > >
> > >
> > > I don't think this is correct. The sensor needs a clock to work.
> > >
> > > With this patch clock_speed which is used to calculate
> > > the framerate is defaulted to 30MHz, crippling all the calculations if
> > > that default value doesn't match what is actually installed on the
> > > board.
> >
> > How come? I kept this:
> >
> > + info->clock_speed = clk_get_rate(info->clk) / 100;
>
> Yes, but only if
> if (info->clk) { }
>
> if (!info->clk) the 'clock_speed' variable is defaulted to 30 at the
> beginning of the probe routine. Am I missing something obvious here?
Maybe. Or I am.
I thought you care about the situation where you *have* the clk, and
thus you shouldn't be caring about the defaults?
> > > If this patch breaks the OLPC, then might it be the DTS for said
> > > device needs to be fixed instead of working around the issue here?
> >
> > No. Device tree is an ABI, and you can't just add mandatory properties.
> >
>
> Well, as I read the ov7670 bindings documentation:
>
> Required Properties:
> - compatible: should be "ovti,ov7670"
> - clocks: reference to the xclk input clock.
> - clock-names: should be "xclk".
>
> It was mandatory already since the bindings have been first created:
> bba582894a ("[media] ov7670: document device tree bindings")
>
> And yes, bindings establishes an ABI we have not to break or make
> incompatible with DTs created for an older version of the same binding,
> but the DTs itself is free to change and we need to do so to update
> it when required (to fix bugs, add new components, enable/disable them
> etc).
Ah, right, you're correct. No DTS ABI breakage there. I guess it would
be fine to revert my patch if we provide the xclk on the OLPC instead.
> > There's no DTS for OLPC XO-1 either; it's an OpenFirmware machine.
> >
>
> I thought OLPC was an ARM machine, that's why I mentioned DTS. Sorry
> about this.
Well, you're sort of right here. The XO-1.75 generation is ARM, the XO-
1 is x86. They both use devicetree provided by the firmware. However,
they predate FDT (or the definitions of bindings that are used here for
tht matter) and the trees are unfortunately quite incomplete. The
sensor is not there.
> A quick read of the wikipedia page for "OpenFirmware" gives me back
> that it a standardized firmware interface:
> "Open Firmware allows the system to load platform-independent drivers
> directly from the PCI card, improving compatibility".
>
> I know nothing on this, and that's not the point, so I'll better stop
> here and refrain to express how much the "loading platform-independent
> (BINARY) drivers from the PCI card" scares me :p
>
> > You'd need to update all machines in the wild which is not realistic.
>
> Machines which have received a kernel update which includes the patch
> that makes the clock for the sensor driver mandatory [1], will have their
> board files updated by the same kernel update, with the proper clock
> provider instantiated for that sensor.
>
> That's what I would expect from a kernel update for those devices (or
> any device in general..)
>
> If this didn't happen, blame OLPC kernel maintainers :p
>
> [1] 0a024d634cee ("[media] ov7670: get xclk"); which went in v4.12
>
> > Alternatively, something else than DT could provide the clock. If this
> > gets in, then the OLPC would work even without the xclk patch:
> > https://lore.kernel.org/lkml/20181105073054.24407-12-lkund...@v3.sk/
>
> That's what I meant, more or less.
>
> If you don't have a DTS you have a board file, isn't it?
> ( arch/x86/platform/olpc/ maybe? )
The device tree on XO-1 is not constructed from a FDT, it's gotten from
the OpenFirmware. See arch/x86/platform/olpc/olpc_dt.c
But as
Re: [PATCH] [media] ov7670: make "xclk" clock optional
Hello,
On Mon, 2018-11-05 at 11:58 +0100, jacopo mondi wrote:
> Hi Lubomir,
>+Sakari in Cc
>
> I just noticed this, and the patch is now in v4.20, but let me comment
> anyway on this.
>
> On Thu, Oct 04, 2018 at 11:29:03PM +0200, Lubomir Rintel wrote:
> > When the "xclk" clock was added, it was made mandatory. This broke the
> > driver on an OLPC plaform which doesn't know such clock. Make it
> > optional.
> >
>
> I don't think this is correct. The sensor needs a clock to work.
>
> With this patch clock_speed which is used to calculate
> the framerate is defaulted to 30MHz, crippling all the calculations if
> that default value doesn't match what is actually installed on the
> board.
How come? I kept this:
+ info->clock_speed = clk_get_rate(info->clk) / 100;
>
> If this patch breaks the OLPC, then might it be the DTS for said
> device needs to be fixed instead of working around the issue here?
No. Device tree is an ABI, and you can't just add mandatory properties.
There's no DTS for OLPC XO-1 either; it's an OpenFirmware machine.
You'd need to update all machines in the wild which is not realistic.
Alternatively, something else than DT could provide the clock. If this
gets in, then the OLPC would work even without the xclk patch:
https://lore.kernel.org/lkml/20181105073054.24407-12-lkund...@v3.sk/
(I just got a kbuild failure message, so I'll surely be following up
with a v2.)
> Also, the DT bindings should be updated too if we decide this property
> can be omitted. At this point, with a follow-up patch.
Yes.
>
> Thanks
Cheers
Lubo
>j
>
> > Tested on a OLPC XO-1 laptop.
> >
> > Cc: sta...@vger.kernel.org # 4.11+
> > Fixes: 0a024d634cee ("[media] ov7670: get xclk")
> > Signed-off-by: Lubomir Rintel
> > ---
> > drivers/media/i2c/ov7670.c | 27 +--
> > 1 file changed, 17 insertions(+), 10 deletions(-)
> >
> > diff --git a/drivers/media/i2c/ov7670.c b/drivers/media/i2c/ov7670.c
> > index 31bf577b0bd3..64d1402882c8 100644
> > --- a/drivers/media/i2c/ov7670.c
> > +++ b/drivers/media/i2c/ov7670.c
> > @@ -1808,17 +1808,24 @@ static int ov7670_probe(struct i2c_client *client,
> > info->pclk_hb_disable = true;
> > }
> >
> > - info->clk = devm_clk_get(&client->dev, "xclk");
> > - if (IS_ERR(info->clk))
> > - return PTR_ERR(info->clk);
> > - ret = clk_prepare_enable(info->clk);
> > - if (ret)
> > - return ret;
> > + info->clk = devm_clk_get(&client->dev, "xclk"); /* optional */
> > + if (IS_ERR(info->clk)) {
> > + ret = PTR_ERR(info->clk);
> > + if (ret == -ENOENT)
> > + info->clk = NULL;
> > + else
> > + return ret;
> > + }
> > + if (info->clk) {
> > + ret = clk_prepare_enable(info->clk);
> > + if (ret)
> > + return ret;
> >
> > - info->clock_speed = clk_get_rate(info->clk) / 100;
> > - if (info->clock_speed < 10 || info->clock_speed > 48) {
> > - ret = -EINVAL;
> > - goto clk_disable;
> > + info->clock_speed = clk_get_rate(info->clk) / 100;
> > + if (info->clock_speed < 10 || info->clock_speed > 48) {
> > + ret = -EINVAL;
> > + goto clk_disable;
> > + }
> > }
> >
> > ret = ov7670_init_gpio(client, info);
> > --
> > 2.19.0
> >
[PATCH 03/11] media: dt-bindings: marvell,mmp2-ccic: Add Marvell MMP2 camera
Add Marvell MMP2 camera host interface.
Signed-off-by: Lubomir Rintel
---
.../bindings/media/marvell,mmp2-ccic.txt | 30 +++
1 file changed, 30 insertions(+)
create mode 100644
Documentation/devicetree/bindings/media/marvell,mmp2-ccic.txt
diff --git a/Documentation/devicetree/bindings/media/marvell,mmp2-ccic.txt
b/Documentation/devicetree/bindings/media/marvell,mmp2-ccic.txt
new file mode 100644
index ..a9c536e58dda
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/marvell,mmp2-ccic.txt
@@ -0,0 +1,30 @@
+Marvell MMP2 camera host interface
+
+Required properties:
+ - compatible: Should be "marvell,mmp2-ccic"
+ - reg: register base and size
+ - interrupts: the interrupt number
+ - any required generic properties defined in video-interfaces.txt
+
+Optional properties:
+ - clocks: input clock (see clock-bindings.txt)
+ - clock-output-names: should contain the name of the clock driving the
+ sensor master clock MCLK
+
+Example:
+
+ camera0: camera@d420a000 {
+ compatible = "marvell,mmp2-ccic";
+ reg = <0xd420a000 0x800>;
+ interrupts = <42>;
+ clocks = <&soc_clocks MMP2_CLK_CCIC0>;
+ clock-names = "CCICAXICLK";
+ #clock-cells = <0>;
+ clock-output-names = "mclk";
+
+ port {
+ camera0_0: endpoint {
+ remote-endpoint = <&ov7670_0>;
+ };
+ };
+ };
--
2.19.1
[PATCH 08/11] [media] marvell-ccic/mmp: enable clock before accessing registers
The access to REG_CLKCTRL or REG_CTRL1 without the clock enabled hangs the machine. Enable the clock first. Signed-off-by: Lubomir Rintel --- drivers/media/platform/marvell-ccic/mmp-driver.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/drivers/media/platform/marvell-ccic/mmp-driver.c b/drivers/media/platform/marvell-ccic/mmp-driver.c index 9e988e527b0d..9c0238f72c40 100644 --- a/drivers/media/platform/marvell-ccic/mmp-driver.c +++ b/drivers/media/platform/marvell-ccic/mmp-driver.c @@ -145,6 +145,7 @@ static int mmpcam_power_up(struct mcam_camera *mcam) * Turn on power and clocks to the controller. */ mmpcam_power_up_ctlr(cam); + mcam_clk_enable(mcam); /* * Provide power to the sensor. */ @@ -158,8 +159,6 @@ static int mmpcam_power_up(struct mcam_camera *mcam) gpio_set_value(pdata->sensor_reset_gpio, 1); /* reset is active low */ mdelay(5); - mcam_clk_enable(mcam); - return 0; } -- 2.19.1
[PATCH 05/11] [media] marvell-ccic: don't generate EOF on parallel bus
The commit 05fed81625bf ("[media] marvell-ccic: add MIPI support for
marvell-ccic driver") that claimed to add CSI2 turned on C0_EOF_VSYNC for
parallel bus without a very good explanation.
That broke camera on OLPC XO-1.75 which precisely uses a sensor on a
parallel bus. Revert that chunk.
Tested on an OLPC XO-1.75.
Fixes: 05fed81625bf755cc67c5864cdfd18b69ea828d1
Signed-off-by: Lubomir Rintel
---
drivers/media/platform/marvell-ccic/mcam-core.c | 6 --
1 file changed, 6 deletions(-)
diff --git a/drivers/media/platform/marvell-ccic/mcam-core.c
b/drivers/media/platform/marvell-ccic/mcam-core.c
index d97f39bde9bd..d24e5b7a3bc5 100644
--- a/drivers/media/platform/marvell-ccic/mcam-core.c
+++ b/drivers/media/platform/marvell-ccic/mcam-core.c
@@ -792,12 +792,6 @@ static void mcam_ctlr_image(struct mcam_camera *cam)
* Make sure it knows we want to use hsync/vsync.
*/
mcam_reg_write_mask(cam, REG_CTRL0, C0_SIF_HVSYNC, C0_SIFM_MASK);
- /*
-* This field controls the generation of EOF(DVP only)
-*/
- if (cam->bus_type != V4L2_MBUS_CSI2_DPHY)
- mcam_reg_set_bit(cam, REG_CTRL0,
- C0_EOF_VSYNC | C0_VEDGE_CTRL);
}
--
2.19.1
[PATCH 06/11] [media] marvell-ccic: drop ctlr_reset()
This accesses the clock registers directly and thus is not too
devicetree friendly. If it's actually needed it needs to be done
differently.
Signed-off-by: Lubomir Rintel
---
.../media/platform/marvell-ccic/mcam-core.c | 6 -
.../media/platform/marvell-ccic/mcam-core.h | 1 -
.../media/platform/marvell-ccic/mmp-driver.c | 23 ---
3 files changed, 30 deletions(-)
diff --git a/drivers/media/platform/marvell-ccic/mcam-core.c
b/drivers/media/platform/marvell-ccic/mcam-core.c
index d24e5b7a3bc5..1b879035948c 100644
--- a/drivers/media/platform/marvell-ccic/mcam-core.c
+++ b/drivers/media/platform/marvell-ccic/mcam-core.c
@@ -1154,12 +1154,6 @@ static void mcam_vb_stop_streaming(struct vb2_queue *vq)
if (cam->state != S_STREAMING)
return;
mcam_ctlr_stop_dma(cam);
- /*
-* Reset the CCIC PHY after stopping streaming,
-* otherwise, the CCIC may be unstable.
-*/
- if (cam->ctlr_reset)
- cam->ctlr_reset(cam);
/*
* VB2 reclaims the buffers, so we need to forget
* about them.
diff --git a/drivers/media/platform/marvell-ccic/mcam-core.h
b/drivers/media/platform/marvell-ccic/mcam-core.h
index ad8955f9f0a1..f93f23faf364 100644
--- a/drivers/media/platform/marvell-ccic/mcam-core.h
+++ b/drivers/media/platform/marvell-ccic/mcam-core.h
@@ -137,7 +137,6 @@ struct mcam_camera {
int (*plat_power_up) (struct mcam_camera *cam);
void (*plat_power_down) (struct mcam_camera *cam);
void (*calc_dphy) (struct mcam_camera *cam);
- void (*ctlr_reset) (struct mcam_camera *cam);
/*
* Everything below here is private to the mcam core and
diff --git a/drivers/media/platform/marvell-ccic/mmp-driver.c
b/drivers/media/platform/marvell-ccic/mmp-driver.c
index 70a2833db0d1..92a79ad8a12c 100644
--- a/drivers/media/platform/marvell-ccic/mmp-driver.c
+++ b/drivers/media/platform/marvell-ccic/mmp-driver.c
@@ -183,28 +183,6 @@ static void mmpcam_power_down(struct mcam_camera *mcam)
mcam_clk_disable(mcam);
}
-static void mcam_ctlr_reset(struct mcam_camera *mcam)
-{
- unsigned long val;
- struct mmp_camera *cam = mcam_to_cam(mcam);
-
- if (mcam->ccic_id) {
- /*
-* Using CCIC2
-*/
- val = ioread32(cam->power_regs + REG_CCIC2_CRCR);
- iowrite32(val & ~0x2, cam->power_regs + REG_CCIC2_CRCR);
- iowrite32(val | 0x2, cam->power_regs + REG_CCIC2_CRCR);
- } else {
- /*
-* Using CCIC1
-*/
- val = ioread32(cam->power_regs + REG_CCIC_CRCR);
- iowrite32(val & ~0x2, cam->power_regs + REG_CCIC_CRCR);
- iowrite32(val | 0x2, cam->power_regs + REG_CCIC_CRCR);
- }
-}
-
/*
* calc the dphy register values
* There are three dphy registers being used.
@@ -352,7 +330,6 @@ static int mmpcam_probe(struct platform_device *pdev)
mcam = &cam->mcam;
mcam->plat_power_up = mmpcam_power_up;
mcam->plat_power_down = mmpcam_power_down;
- mcam->ctlr_reset = mcam_ctlr_reset;
mcam->calc_dphy = mmpcam_calc_dphy;
mcam->dev = &pdev->dev;
mcam->use_smbus = 0;
--
2.19.1
[PATCH 01/11] media: ov7670: hook s_power onto v4l2 core
The commit 71862f63f351 ("media: ov7670: Add the ov7670_s_power function")
added a s_power function. For some reason it didn't register it with v4l2,
only uses it internally. Fix this now.
Signed-off-by: Lubomir Rintel
---
drivers/media/i2c/ov7670.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/media/i2c/ov7670.c b/drivers/media/i2c/ov7670.c
index bc68a3a5b4ec..d87f2362bf40 100644
--- a/drivers/media/i2c/ov7670.c
+++ b/drivers/media/i2c/ov7670.c
@@ -1651,6 +1651,7 @@ static int ov7670_open(struct v4l2_subdev *sd, struct
v4l2_subdev_fh *fh)
static const struct v4l2_subdev_core_ops ov7670_core_ops = {
.reset = ov7670_reset,
.init = ov7670_init,
+ .s_power = ov7670_s_power,
#ifdef CONFIG_VIDEO_ADV_DEBUG
.g_register = ov7670_g_register,
.s_register = ov7670_s_register,
--
2.19.1
[PATCH 0/11] media: make Marvell camera work on DT-based OLPC XO-1.75
Hi, this patch set somewhat modernizes the Marvel MMP2 CCIC driver. Notably, it ports it from the platform data (which seems unused as the board support code never made it) to devicetree. At the core of the rework is the move to asynchronous sensor discovery and clock management with the standard clock framework. There are also some straightforward fixes for bitrotten parts. There's probably still room for improvement, but as it is, it seems to work well on OLPC XO-1.75 and doesn't break OLPC XO-1 (I've tested on both platforms). Cheers, Lubo
[PATCH 02/11] media: ov7670: control clock along with power
This provides more power saving when the sensor is off.
While at that, do the delay on power/clock enable even if the sensor driver
itself doesn't control the GPIOs. This is required for the OLPC XO-1
platform, that lacks the proper power/reset properties in its DT, but
needs the delay after the sensor is clocked up.
Signed-off-by: Lubomir Rintel
---
drivers/media/i2c/ov7670.c | 37 +
1 file changed, 21 insertions(+), 16 deletions(-)
diff --git a/drivers/media/i2c/ov7670.c b/drivers/media/i2c/ov7670.c
index d87f2362bf40..a3e72c62382c 100644
--- a/drivers/media/i2c/ov7670.c
+++ b/drivers/media/i2c/ov7670.c
@@ -241,6 +241,7 @@ struct ov7670_info {
struct v4l2_mbus_framefmt format;
struct ov7670_format_struct *fmt; /* Current format */
struct clk *clk;
+ int on;
struct gpio_desc *resetb_gpio;
struct gpio_desc *pwdn_gpio;
unsigned int mbus_config; /* Media bus configuration flags */
@@ -1610,15 +1611,26 @@ static int ov7670_s_power(struct v4l2_subdev *sd, int
on)
{
struct ov7670_info *info = to_state(sd);
+ if (info->on == on)
+ return 0;
+
+ if (on)
+ clk_prepare_enable(info->clk);
+ else
+ clk_disable_unprepare(info->clk);
+
if (info->pwdn_gpio)
gpiod_set_value(info->pwdn_gpio, !on);
if (on && info->resetb_gpio) {
gpiod_set_value(info->resetb_gpio, 1);
usleep_range(500, 1000);
gpiod_set_value(info->resetb_gpio, 0);
- usleep_range(3000, 5000);
}
+ if (on && (info->pwdn_gpio || info->resetb_gpio || info->clk))
+ usleep_range(3000, 5000);
+
+ info->on = on;
return 0;
}
@@ -1817,24 +1829,20 @@ static int ov7670_probe(struct i2c_client *client,
else
return ret;
}
- if (info->clk) {
- ret = clk_prepare_enable(info->clk);
- if (ret)
- return ret;
+ ret = ov7670_init_gpio(client, info);
+ if (ret)
+ return ret;
+
+ ov7670_s_power(sd, 1);
+ if (info->clk) {
info->clock_speed = clk_get_rate(info->clk) / 100;
if (info->clock_speed < 10 || info->clock_speed > 48) {
ret = -EINVAL;
- goto clk_disable;
+ goto power_off;
}
}
- ret = ov7670_init_gpio(client, info);
- if (ret)
- goto clk_disable;
-
- ov7670_s_power(sd, 1);
-
/* Make sure it's an ov7670 */
ret = ov7670_detect(sd);
if (ret) {
@@ -1913,6 +1921,7 @@ static int ov7670_probe(struct i2c_client *client,
if (ret < 0)
goto entity_cleanup;
+ ov7670_s_power(sd, 0);
return 0;
entity_cleanup:
@@ -1921,12 +1930,9 @@ static int ov7670_probe(struct i2c_client *client,
v4l2_ctrl_handler_free(&info->hdl);
power_off:
ov7670_s_power(sd, 0);
-clk_disable:
- clk_disable_unprepare(info->clk);
return ret;
}
-
static int ov7670_remove(struct i2c_client *client)
{
struct v4l2_subdev *sd = i2c_get_clientdata(client);
@@ -1934,7 +1940,6 @@ static int ov7670_remove(struct i2c_client *client)
v4l2_async_unregister_subdev(sd);
v4l2_ctrl_handler_free(&info->hdl);
- clk_disable_unprepare(info->clk);
media_entity_cleanup(&info->sd.entity);
ov7670_s_power(sd, 0);
return 0;
--
2.19.1
[PATCH 09/11] [media] marvell-ccic/mmp: add devicetree support
The platform data is actually not used anywhere (along with the CSI
support) and should be safe to remove.
Signed-off-by: Lubomir Rintel
---
.../media/platform/marvell-ccic/mmp-driver.c | 36 ++-
1 file changed, 27 insertions(+), 9 deletions(-)
diff --git a/drivers/media/platform/marvell-ccic/mmp-driver.c
b/drivers/media/platform/marvell-ccic/mmp-driver.c
index 9c0238f72c40..ff1feca7bc9b 100644
--- a/drivers/media/platform/marvell-ccic/mmp-driver.c
+++ b/drivers/media/platform/marvell-ccic/mmp-driver.c
@@ -19,6 +19,8 @@
#include
#include
#include
+#include
+#include
#include
#include
#include
@@ -197,6 +199,9 @@ static void mmpcam_calc_dphy(struct mcam_camera *mcam)
struct device *dev = &cam->pdev->dev;
unsigned long tx_clk_esc;
+ if (!pdata)
+ return;
+
/*
* If CSI2_DPHY3 is calculated dynamically,
* pdata->lane_clk should be already set
@@ -315,10 +320,6 @@ static int mmpcam_probe(struct platform_device *pdev)
struct mmp_camera_platform_data *pdata;
int ret;
- pdata = pdev->dev.platform_data;
- if (!pdata)
- return -ENODEV;
-
cam = devm_kzalloc(&pdev->dev, sizeof(*cam), GFP_KERNEL);
if (cam == NULL)
return -ENOMEM;
@@ -331,17 +332,29 @@ static int mmpcam_probe(struct platform_device *pdev)
mcam->calc_dphy = mmpcam_calc_dphy;
mcam->dev = &pdev->dev;
mcam->use_smbus = 0;
- mcam->mclk_src = pdata->mclk_src;
- mcam->mclk_div = pdata->mclk_div;
- mcam->bus_type = pdata->bus_type;
- mcam->dphy = pdata->dphy;
+ pdata = pdev->dev.platform_data;
+ if (pdata) {
+ mcam->mclk_src = pdata->mclk_src;
+ mcam->mclk_div = pdata->mclk_div;
+ mcam->bus_type = pdata->bus_type;
+ mcam->dphy = pdata->dphy;
+ mcam->lane = pdata->lane;
+ } else {
+ /*
+* This are values that used to be hardcoded in mcam-core and
+* work well on a OLPC XO 1.75 with a parallel bus sensor.
+* If it turns out other setups make sense, the values should
+* be obtained from the device tree.
+*/
+ mcam->mclk_src = 3;
+ mcam->mclk_div = 2;
+ }
if (mcam->bus_type == V4L2_MBUS_CSI2_DPHY) {
cam->mipi_clk = devm_clk_get(mcam->dev, "mipi");
if ((IS_ERR(cam->mipi_clk) && mcam->dphy[2] == 0))
return PTR_ERR(cam->mipi_clk);
}
mcam->mipi_enabled = false;
- mcam->lane = pdata->lane;
mcam->chip_id = MCAM_ARMADA610;
mcam->buffer_mode = B_DMA_sg;
strscpy(mcam->bus_info, "platform:mmp-camera", sizeof(mcam->bus_info));
@@ -476,6 +489,10 @@ static int mmpcam_resume(struct platform_device *pdev)
#endif
+static const struct of_device_id mmpcam_of_match[] = {
+ { .compatible = "marvell,mmp2-ccic", },
+ {},
+};
static struct platform_driver mmpcam_driver = {
.probe = mmpcam_probe,
@@ -486,6 +503,7 @@ static struct platform_driver mmpcam_driver = {
#endif
.driver = {
.name = "mmp-camera",
+ .of_match_table = of_match_ptr(mmpcam_of_match),
}
};
--
2.19.1
[PATCH 07/11] [media] marvell-ccic: drop unused stuff
Remove structure members and headers that are not actually used. Saves
us from some noise in subsequent cleanup commits.
Signed-off-by: Lubomir Rintel
---
drivers/media/platform/marvell-ccic/mcam-core.c | 1 -
drivers/media/platform/marvell-ccic/mcam-core.h | 3 ---
drivers/media/platform/marvell-ccic/mmp-driver.c | 3 ---
include/linux/platform_data/media/mmp-camera.h | 1 -
4 files changed, 8 deletions(-)
diff --git a/drivers/media/platform/marvell-ccic/mcam-core.c
b/drivers/media/platform/marvell-ccic/mcam-core.c
index 1b879035948c..0113b8d37d03 100644
--- a/drivers/media/platform/marvell-ccic/mcam-core.c
+++ b/drivers/media/platform/marvell-ccic/mcam-core.c
@@ -1776,7 +1776,6 @@ int mccic_register(struct mcam_camera *cam)
*/
sensor_cfg.clock_speed = cam->clock_speed;
sensor_cfg.use_smbus = cam->use_smbus;
- cam->sensor_addr = ov7670_info.addr;
cam->sensor = v4l2_i2c_new_subdev_board(&cam->v4l2_dev,
cam->i2c_adapter, &ov7670_info, NULL);
if (cam->sensor == NULL) {
diff --git a/drivers/media/platform/marvell-ccic/mcam-core.h
b/drivers/media/platform/marvell-ccic/mcam-core.h
index f93f23faf364..b828b1bb59d3 100644
--- a/drivers/media/platform/marvell-ccic/mcam-core.h
+++ b/drivers/media/platform/marvell-ccic/mcam-core.h
@@ -112,11 +112,9 @@ struct mcam_camera {
short int use_smbus;/* SMBUS or straight I2c? */
enum mcam_buffer_mode buffer_mode;
- int mclk_min; /* The minimal value of mclk */
int mclk_src; /* which clock source the mclk derives from */
int mclk_div; /* Clock Divider Value for MCLK */
- int ccic_id;
enum v4l2_mbus_type bus_type;
/* MIPI support */
/* The dphy config value, allocated in board file
@@ -153,7 +151,6 @@ struct mcam_camera {
*/
struct video_device vdev;
struct v4l2_subdev *sensor;
- unsigned short sensor_addr;
/* Videobuf2 stuff */
struct vb2_queue vb_queue;
diff --git a/drivers/media/platform/marvell-ccic/mmp-driver.c
b/drivers/media/platform/marvell-ccic/mmp-driver.c
index 92a79ad8a12c..9e988e527b0d 100644
--- a/drivers/media/platform/marvell-ccic/mmp-driver.c
+++ b/drivers/media/platform/marvell-ccic/mmp-driver.c
@@ -12,7 +12,6 @@
#include
#include
#include
-#include
#include
#include
#include
@@ -333,8 +332,6 @@ static int mmpcam_probe(struct platform_device *pdev)
mcam->calc_dphy = mmpcam_calc_dphy;
mcam->dev = &pdev->dev;
mcam->use_smbus = 0;
- mcam->ccic_id = pdev->id;
- mcam->mclk_min = pdata->mclk_min;
mcam->mclk_src = pdata->mclk_src;
mcam->mclk_div = pdata->mclk_div;
mcam->bus_type = pdata->bus_type;
diff --git a/include/linux/platform_data/media/mmp-camera.h
b/include/linux/platform_data/media/mmp-camera.h
index d2d3a443eedf..4c3a80a45883 100644
--- a/include/linux/platform_data/media/mmp-camera.h
+++ b/include/linux/platform_data/media/mmp-camera.h
@@ -16,7 +16,6 @@ struct mmp_camera_platform_data {
int sensor_power_gpio;
int sensor_reset_gpio;
enum v4l2_mbus_type bus_type;
- int mclk_min; /* The minimal value of MCLK */
int mclk_src; /* which clock source the MCLK derives from */
int mclk_div; /* Clock Divider Value for MCLK */
/*
--
2.19.1
[PATCH 10/11] [media] marvell-ccic: use async notifier to get the sensor
An instance of a sensor on DT-based MMP2 platform is always going to be
created asynchronously.
Let's move the manual device creation away from the core to the Cafe
driver (used on OLPC XO-1, not present in DT) and set up appropriate
async matches: I2C on Cafe, FWNODE on MMP (OLPC XO-1.75).
Signed-off-by: Lubomir Rintel
---
.../media/platform/marvell-ccic/cafe-driver.c | 49 --
.../media/platform/marvell-ccic/mcam-core.c | 157 --
.../media/platform/marvell-ccic/mcam-core.h | 5 +-
.../media/platform/marvell-ccic/mmp-driver.c | 27 +--
.../linux/platform_data/media/mmp-camera.h| 1 -
5 files changed, 162 insertions(+), 77 deletions(-)
diff --git a/drivers/media/platform/marvell-ccic/cafe-driver.c
b/drivers/media/platform/marvell-ccic/cafe-driver.c
index 2986cb4b88d0..0164afc405d1 100644
--- a/drivers/media/platform/marvell-ccic/cafe-driver.c
+++ b/drivers/media/platform/marvell-ccic/cafe-driver.c
@@ -8,6 +8,7 @@
*
* Copyright 2006-11 One Laptop Per Child Association, Inc.
* Copyright 2006-11 Jonathan Corbet
+ * Copyright 2018 Lubomir Rintel
*
* Written by Jonathan Corbet, cor...@lwn.net.
*
@@ -27,6 +28,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -52,6 +54,7 @@ struct cafe_camera {
int registered; /* Fully initialized? */
struct mcam_camera mcam;
struct pci_dev *pdev;
+ struct i2c_adapter *i2c_adapter;
wait_queue_head_t smbus_wait; /* Waiting on i2c events */
};
@@ -351,15 +354,15 @@ static int cafe_smbus_setup(struct cafe_camera *cam)
return ret;
}
- cam->mcam.i2c_adapter = adap;
+ cam->i2c_adapter = adap;
cafe_smbus_enable_irq(cam);
return 0;
}
static void cafe_smbus_shutdown(struct cafe_camera *cam)
{
- i2c_del_adapter(cam->mcam.i2c_adapter);
- kfree(cam->mcam.i2c_adapter);
+ i2c_del_adapter(cam->i2c_adapter);
+ kfree(cam->i2c_adapter);
}
@@ -452,6 +455,29 @@ static irqreturn_t cafe_irq(int irq, void *data)
return IRQ_RETVAL(handled);
}
+/* --
*/
+
+static struct ov7670_config sensor_cfg = {
+ /*
+* Exclude QCIF mode, because it only captures a tiny portion
+* of the sensor FOV
+*/
+ .min_width = 320,
+ .min_height = 240,
+
+ /*
+* Set the clock speed for the XO 1; I don't believe this
+* driver has ever run anywhere else.
+*/
+ .clock_speed = 45,
+ .use_smbus = 1,
+};
+
+struct i2c_board_info ov7670_info = {
+ .type = "ov7670",
+ .addr = 0x42 >> 1,
+ .platform_data = &sensor_cfg,
+};
/* --
*/
/*
@@ -481,12 +507,6 @@ static int cafe_pci_probe(struct pci_dev *pdev,
mcam->plat_power_down = cafe_ctlr_power_down;
mcam->dev = &pdev->dev;
snprintf(mcam->bus_info, sizeof(mcam->bus_info), "PCI:%s",
pci_name(pdev));
- /*
-* Set the clock speed for the XO 1; I don't believe this
-* driver has ever run anywhere else.
-*/
- mcam->clock_speed = 45;
- mcam->use_smbus = 1;
/*
* Vmalloc mode for buffers is traditional with this driver.
* We *might* be able to run DMA_contig, especially on a system
@@ -527,12 +547,21 @@ static int cafe_pci_probe(struct pci_dev *pdev,
if (ret)
goto out_pdown;
+ mcam->asd.match_type = V4L2_ASYNC_MATCH_I2C;
+ mcam->asd.match.i2c.adapter_id = i2c_adapter_id(cam->i2c_adapter);
+ mcam->asd.match.i2c.address = ov7670_info.addr;
+
ret = mccic_register(mcam);
- if (ret == 0) {
+ if (ret)
+ goto out_smbus_shutdown;
+
+ if (i2c_new_device(cam->i2c_adapter, &ov7670_info)) {
cam->registered = 1;
return 0;
}
+ cafe_shutdown(cam);
+out_smbus_shutdown:
cafe_smbus_shutdown(cam);
out_pdown:
cafe_ctlr_power_down(mcam);
diff --git a/drivers/media/platform/marvell-ccic/mcam-core.c
b/drivers/media/platform/marvell-ccic/mcam-core.c
index 0113b8d37d03..87812b7287f0 100644
--- a/drivers/media/platform/marvell-ccic/mcam-core.c
+++ b/drivers/media/platform/marvell-ccic/mcam-core.c
@@ -4,6 +4,7 @@
* so it needs platform-specific support outside of the core.
*
* Copyright 2011 Jonathan Corbet cor...@lwn.net
+ * Copyright 2018 Lubomir Rintel
*/
#include
#include
@@ -26,7 +27,6 @@
#include
#include
#include
-#include
#include
#include
#include
@@ -93,6 +93,9 @@ MODULE_PARM_DESC(buffer_mode,
#define sensor_call(cam, o, f, args...) \
v4l2_subdev_call(cam->sensor, o, f, ##args)
+#define notifier_to_mcam(notifier) \
+ container_o
[PATCH 04/11] [media] marvell-ccic: fix DMA s/g desc number calculation
The commit d790b7eda953 ("[media] vb2-dma-sg: move dma_(un)map_sg here")
left dma_desc_nent unset. It previously contained the number of DMA
descriptors as returned from dma_map_sg().
We can now (since the commit referred to above) obtain the same value from
the sg_table and drop dma_desc_nent altogether.
Tested on OLPC XO-1.75 machine. Doesn't affect the OLPC XO-1's Cafe
driver, since that one doesn't do DMA.
Fixes: d790b7eda953df474f470169ebdf111c02fa7a2d
Signed-off-by: Lubomir Rintel
---
drivers/media/platform/marvell-ccic/mcam-core.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/media/platform/marvell-ccic/mcam-core.c
b/drivers/media/platform/marvell-ccic/mcam-core.c
index f1b301810260..d97f39bde9bd 100644
--- a/drivers/media/platform/marvell-ccic/mcam-core.c
+++ b/drivers/media/platform/marvell-ccic/mcam-core.c
@@ -200,7 +200,6 @@ struct mcam_vb_buffer {
struct list_head queue;
struct mcam_dma_desc *dma_desc; /* Descriptor virtual address */
dma_addr_t dma_desc_pa; /* Descriptor physical address */
- int dma_desc_nent; /* Number of mapped descriptors */
};
static inline struct mcam_vb_buffer *vb_to_mvb(struct vb2_v4l2_buffer *vb)
@@ -608,9 +607,11 @@ static void mcam_dma_contig_done(struct mcam_camera *cam,
int frame)
static void mcam_sg_next_buffer(struct mcam_camera *cam)
{
struct mcam_vb_buffer *buf;
+ struct sg_table *sg_table;
buf = list_first_entry(&cam->buffers, struct mcam_vb_buffer, queue);
list_del_init(&buf->queue);
+ sg_table = vb2_dma_sg_plane_desc(&buf->vb_buf.vb2_buf, 0);
/*
* Very Bad Not Good Things happen if you don't clear
* C1_DESC_ENA before making any descriptor changes.
@@ -618,7 +619,7 @@ static void mcam_sg_next_buffer(struct mcam_camera *cam)
mcam_reg_clear_bit(cam, REG_CTRL1, C1_DESC_ENA);
mcam_reg_write(cam, REG_DMA_DESC_Y, buf->dma_desc_pa);
mcam_reg_write(cam, REG_DESC_LEN_Y,
- buf->dma_desc_nent*sizeof(struct mcam_dma_desc));
+ sg_table->nents*sizeof(struct mcam_dma_desc));
mcam_reg_write(cam, REG_DESC_LEN_U, 0);
mcam_reg_write(cam, REG_DESC_LEN_V, 0);
mcam_reg_set_bit(cam, REG_CTRL1, C1_DESC_ENA);
--
2.19.1
[PATCH 11/11] [media] marvell-ccic: provide a clock for the sensor
The sensor needs the MCLK clock running when it's being probed. On
platforms where the sensor is instantiated from a DT (MMP2) it is going
to happen asynchronously.
Therefore, the current modus operandi, where the bridge driver fiddles
with the sensor power and clock itself is not going to fly. As the comments
wisely note, this doesn't even belong there.
Luckily, the ov7670 driver is already able to control its power and
reset lines, we can just drop the MMP platform glue altogether.
It also requests the clock via the standard clock subsystem. Good -- let's
set up a clock instance so that the sensor can ask us to enable the clock.
Note that this is pretty dumb at the moment: the clock is hardwired to a
particular frequency and parent. It was always the case.
Signed-off-by: Lubomir Rintel
---
.../media/platform/marvell-ccic/cafe-driver.c | 11 +-
.../media/platform/marvell-ccic/mcam-core.c | 158 +++---
.../media/platform/marvell-ccic/mcam-core.h | 3 +
.../media/platform/marvell-ccic/mmp-driver.c | 153 ++---
.../linux/platform_data/media/mmp-camera.h| 2 -
5 files changed, 161 insertions(+), 166 deletions(-)
diff --git a/drivers/media/platform/marvell-ccic/cafe-driver.c
b/drivers/media/platform/marvell-ccic/cafe-driver.c
index 0164afc405d1..0e712bb941ba 100644
--- a/drivers/media/platform/marvell-ccic/cafe-driver.c
+++ b/drivers/media/platform/marvell-ccic/cafe-driver.c
@@ -33,6 +33,7 @@
#include
#include
#include
+#include
#include "mcam-core.h"
@@ -533,11 +534,10 @@ static int cafe_pci_probe(struct pci_dev *pdev,
goto out_iounmap;
/*
-* Initialize the controller and leave it powered up. It will
-* stay that way until the sensor driver shows up.
+* Initialize the controller.
*/
cafe_ctlr_init(mcam);
- cafe_ctlr_power_up(mcam);
+
/*
* Set up I2C/SMBUS communications. We have to drop the mutex here
* because the sensor could attach in this call chain, leading to
@@ -555,12 +555,15 @@ static int cafe_pci_probe(struct pci_dev *pdev,
if (ret)
goto out_smbus_shutdown;
+ clkdev_create(mcam->mclk, "xclk", "%d-%04x",
+ i2c_adapter_id(cam->i2c_adapter), ov7670_info.addr);
+
if (i2c_new_device(cam->i2c_adapter, &ov7670_info)) {
cam->registered = 1;
return 0;
}
- cafe_shutdown(cam);
+ mccic_shutdown(mcam);
out_smbus_shutdown:
cafe_smbus_shutdown(cam);
out_pdown:
diff --git a/drivers/media/platform/marvell-ccic/mcam-core.c
b/drivers/media/platform/marvell-ccic/mcam-core.c
index 87812b7287f0..4656eaee2f29 100644
--- a/drivers/media/platform/marvell-ccic/mcam-core.c
+++ b/drivers/media/platform/marvell-ccic/mcam-core.c
@@ -22,6 +22,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -303,9 +304,6 @@ static void mcam_enable_mipi(struct mcam_camera *mcam)
*/
mcam_reg_write(mcam, REG_CSI2_CTRL0,
CSI2_C0_MIPI_EN | CSI2_C0_ACT_LANE(mcam->lane));
- mcam_reg_write(mcam, REG_CLKCTRL,
- (mcam->mclk_src << 29) | mcam->mclk_div);
-
mcam->mipi_enabled = true;
}
}
@@ -846,11 +844,6 @@ static void mcam_ctlr_init(struct mcam_camera *cam)
* but it's good to be sure.
*/
mcam_reg_clear_bit(cam, REG_CTRL0, C0_ENABLE);
- /*
-* Clock the sensor appropriately. Controller clock should
-* be 48MHz, sensor "typical" value is half that.
-*/
- mcam_reg_write_mask(cam, REG_CLKCTRL, 2, CLK_DIV_MASK);
spin_unlock_irqrestore(&cam->dev_lock, flags);
}
@@ -898,14 +891,15 @@ static int mcam_ctlr_power_up(struct mcam_camera *cam)
int ret;
spin_lock_irqsave(&cam->dev_lock, flags);
- ret = cam->plat_power_up(cam);
- if (ret) {
- spin_unlock_irqrestore(&cam->dev_lock, flags);
- return ret;
+ if (cam->plat_power_up) {
+ ret = cam->plat_power_up(cam);
+ if (ret) {
+ spin_unlock_irqrestore(&cam->dev_lock, flags);
+ return ret;
+ }
}
mcam_reg_clear_bit(cam, REG_CTRL1, C1_PWRDWN);
spin_unlock_irqrestore(&cam->dev_lock, flags);
- msleep(5); /* Just to be sure */
return 0;
}
@@ -920,10 +914,101 @@ static void mcam_ctlr_power_down(struct mcam_camera *cam)
* power down routine.
*/
mcam_reg_set_bit(cam, REG_CTRL1, C1_PWRDWN);
- cam->plat_power_down(cam);
+ if (cam->plat_power_down)
+ cam->plat_power_down(cam);
spin_unlock_irqrestore(&cam->dev_lock, flags);
}
+/* -
Re: [PATCH] media: usbtv: fix brightness and contrast controls
On Tue, 2017-10-24 at 21:14 +0100, Adam Sampson wrote:
> Because the brightness and contrast controls share a register,
> usbtv_s_ctrl needs to read the existing values for both controls
> before
> inserting the new value. However, the code accidentally wrote to the
> registers (from an uninitialised stack array), rather than reading
> them.
>
> The user-visible effect of this was that adjusting the brightness
> would
> also set the contrast to a random value, and vice versa -- so it
> wasn't
> possible to correctly adjust the brightness of usbtv's video output.
>
> Tested with an "EasyDAY" UTV007 device.
>
> Fixes: c53a846c48f2 ("usbtv: add video controls")
> Signed-off-by: Adam Sampson
Thank you!
Reviewed-By: Lubomir Rintel
> ---
> drivers/media/usb/usbtv/usbtv-video.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/media/usb/usbtv/usbtv-video.c
> b/drivers/media/usb/usbtv/usbtv-video.c
> index 95b5f43..3668a04 100644
> --- a/drivers/media/usb/usbtv/usbtv-video.c
> +++ b/drivers/media/usb/usbtv/usbtv-video.c
> @@ -718,8 +718,8 @@ static int usbtv_s_ctrl(struct v4l2_ctrl *ctrl)
>*/
> if (ctrl->id == V4L2_CID_BRIGHTNESS || ctrl->id ==
> V4L2_CID_CONTRAST) {
> ret = usb_control_msg(usbtv->udev,
> - usb_sndctrlpipe(usbtv->udev, 0),
> USBTV_CONTROL_REG,
> - USB_DIR_OUT | USB_TYPE_VENDOR |
> USB_RECIP_DEVICE,
> + usb_rcvctrlpipe(usbtv->udev, 0),
> USBTV_CONTROL_REG,
> + USB_DIR_IN | USB_TYPE_VENDOR |
> USB_RECIP_DEVICE,
> 0, USBTV_BASE + 0x0244, (void *)data, 3, 0);
> if (ret < 0)
> goto error;
A patch slipped through the cracks?
Hi, we're trying to get this reasonably trivial patch [1] applied for more than a year and four attempts now. (I'm not including it in this message so that this message won't be ignored for the same reason the submissions were, whatever they are.) [1] https://patchwork.linuxtv.org/patch/40862/ I have no idea what went wrong. There was a suspicion (somewhat confirmed by the initial patch submitter) that spam filtering could have dropped the first message. Since then the patch did make it to the list numerous times and was picked up by patchwork. The patchwork's idea about the patch being "Superseded" is wrong -- I have no idea why. But someone *please* look into this and apply the patch. Thank you Lubo
Re: [PATCH 3/3] media: add V4L2_CAP_VDEV_CENTERED flag on vdev-centric drivers
On Fri, 2017-08-25 at 06:40 -0300, Mauro Carvalho Chehab wrote: > From: Mauro Carvalho Chehab > > Those devices are controlled via their V4L2 device. Add a > flag to indicate them as such. > > Signed-off-by: Mauro Carvalho Chehab > Signed-off-by: Mauro Carvalho Chehab Acked-by: Lubomir Rintel for the usbtv driver. > --- > drivers/media/usb/usbtv/usbtv-video.c | 2 +- > diff --git a/drivers/media/usb/usbtv/usbtv-video.c > b/drivers/media/usb/usbtv/usbtv-video.c > index 8135614f395a..724dbcb8de9b 100644 > --- a/drivers/media/usb/usbtv/usbtv-video.c > +++ b/drivers/media/usb/usbtv/usbtv-video.c > @@ -520,7 +520,7 @@ static int usbtv_querycap(struct file *file, void *priv, > strlcpy(cap->driver, "usbtv", sizeof(cap->driver)); > strlcpy(cap->card, "usbtv", sizeof(cap->card)); > usb_make_path(dev->udev, cap->bus_info, sizeof(cap->bus_info)); > - cap->device_caps = V4L2_CAP_VIDEO_CAPTURE; > + cap->device_caps = V4L2_CAP_VIDEO_CAPTURE | V4L2_CAP_VDEV_CENTERED; > cap->device_caps |= V4L2_CAP_READWRITE | V4L2_CAP_STREAMING; > cap->capabilities = cap->device_caps | V4L2_CAP_DEVICE_CAPS; > return 0; >
[PATCH] usbtv: add a new usbid
From: Icenowy Zheng
A new usbid of UTV007 is found in a newly bought device.
The usbid is 1f71:3301.
The ID on the chip is:
UTV007
A89029.1
1520L18K1
Both video and audio is tested with the modified usbtv driver.
Reviewed-by: Lubomir Rintel
Signed-off-by: Icenowy Zheng
Signed-off-by: Lubomir Rintel
---
I'm resending this one, since the earlier two attempts by the original author
don't seem to have made it to linux-media@ or lkmk@, possibly due to spam
filters or something and thus patchwork didn't pick it up.
drivers/media/usb/usbtv/usbtv-core.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/media/usb/usbtv/usbtv-core.c
b/drivers/media/usb/usbtv/usbtv-core.c
index ceb953b..cae6378 100644
--- a/drivers/media/usb/usbtv/usbtv-core.c
+++ b/drivers/media/usb/usbtv/usbtv-core.c
@@ -144,6 +144,7 @@ static void usbtv_disconnect(struct usb_interface *intf)
static struct usb_device_id usbtv_id_table[] = {
{ USB_DEVICE(0x1b71, 0x3002) },
+ { USB_DEVICE(0x1f71, 0x3301) },
{}
};
MODULE_DEVICE_TABLE(usb, usbtv_id_table);
--
2.9.3
Re: [PATCH RESEND] [media] usbtv: add a new usbid
On Wed, 2017-02-08 at 02:43 +0800, Icenowy Zheng wrote:
> A new usbid of UTV007 is found in a newly bought device.
>
> The usbid is 1f71:3301.
>
> The ID on the chip is:
> UTV007
> A89029.1
> 1520L18K1
>
> Both video and audio is tested with the modified usbtv driver.
>
> Signed-off-by: Icenowy Zheng
Acked-by: Lubomir Rintel
Thanks for resending this. I can't seem to find the original posting in
the patchwork and don't see how could it have slipped through the
cracks. But then my understanding of how does the media tree
maintenance might not be too good.
Also, I think new USB IDs are usually okay for stable trees too, if you
care about that then feel free to Cc stable@ too.
Lubo
> ---
> drivers/media/usb/usbtv/usbtv-core.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/media/usb/usbtv/usbtv-core.c
> b/drivers/media/usb/usbtv/usbtv-core.c
> index ceb953be0770..cae637845876 100644
> --- a/drivers/media/usb/usbtv/usbtv-core.c
> +++ b/drivers/media/usb/usbtv/usbtv-core.c
> @@ -144,6 +144,7 @@ static void usbtv_disconnect(struct usb_interface
> *intf)
>
> static struct usb_device_id usbtv_id_table[] = {
> { USB_DEVICE(0x1b71, 0x3002) },
> + { USB_DEVICE(0x1f71, 0x3301) },
> {}
> };
> MODULE_DEVICE_TABLE(usb, usbtv_id_table);
--
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
[PATCH] [media] usbtv: add sharpness control
Signed-off-by: Lubomir Rintel
---
drivers/media/usb/usbtv/usbtv-video.c | 8
1 file changed, 8 insertions(+)
diff --git a/drivers/media/usb/usbtv/usbtv-video.c
b/drivers/media/usb/usbtv/usbtv-video.c
index d3b6d3d..8135614 100644
--- a/drivers/media/usb/usbtv/usbtv-video.c
+++ b/drivers/media/usb/usbtv/usbtv-video.c
@@ -757,6 +757,12 @@ static int usbtv_s_ctrl(struct v4l2_ctrl *ctrl)
data[1] = -ctrl->val & 0xff;
}
break;
+ case V4L2_CID_SHARPNESS:
+ index = USBTV_BASE + 0x0239;
+ data[0] = 0;
+ data[1] = ctrl->val;
+ size = 2;
+ break;
default:
kfree(data);
return -EINVAL;
@@ -825,6 +831,8 @@ int usbtv_video_init(struct usbtv *usbtv)
V4L2_CID_SATURATION, 0, 0x3ff, 1, 0x200);
v4l2_ctrl_new_std(&usbtv->ctrl, &usbtv_ctrl_ops,
V4L2_CID_HUE, -0xdff, 0xdff, 1, 0x000);
+ v4l2_ctrl_new_std(&usbtv->ctrl, &usbtv_ctrl_ops,
+ V4L2_CID_SHARPNESS, 0x0, 0xff, 1, 0x60);
ret = usbtv->ctrl.error;
if (ret < 0) {
dev_warn(usbtv->dev, "Could not initialize controls\n");
--
2.9.3
--
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
Re: [PATCH] [media] usbtv: add a new usbid
On Mon, 2016-12-05 at 23:47 +0800, Icenowy Zheng wrote: > 2016年12月5日 19:49于 Lubomir Rintel 写道： > > > > On Sun, 2016-12-04 at 22:59 +0800, Icenowy Zheng wrote: > > > > > > 04.12.2016, 22:00, "Icenowy Zheng" : > > > > A new usbid of UTV007 is found in a newly bought device. > > > > > > > > The usbid is 1f71:3301. > > > > > > > > The ID on the chip is: > > > > UTV007 > > > > A89029.1 > > > > 1520L18K1 > > > > > > > > > > Seems that my device come with more capabilities. > > > > > > I tested it under Windows, and I got wireless Analog TV > > > and FM radio functions. (An antenna is shipped with my device) > > > > > > Maybe a new radio function is be added, combined with the > > > new USB ID. > > > > > > But at least Composite AV function works well with current usbtv > > > driver and XawTV. > > > > Well, someone with the hardware would need to capture the traffic > > from > > the Windows driver (and ideally also extend the driver). Would you > > mind > > giving it a try? > > How to do it? > > Use wireshark? Yes, wireshark is okay. I've been using that one. Another good option I discovered recently is usb_capture from usbsniff package. > > Do you have a link to some further details about the device you > > got? > > Perhaps if it's available cheaply from dealextreme or somewhere I > > could > > take a look too. > > I bought directly from Taobao (I'm in China). Do you happen to have a link? Lubo -- 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
Re: [PATCH] [media] usbtv: add a new usbid
On Sun, 2016-12-04 at 21:59 +0800, Icenowy Zheng wrote:
> A new usbid of UTV007 is found in a newly bought device.
>
> The usbid is 1f71:3301.
>
> The ID on the chip is:
> UTV007
> A89029.1
> 1520L18K1
>
> Both video and audio is tested with the modified usbtv driver.
Thank you.
Acked-by: Lubomir Rintel
Also, it may make sense to add
Tested-by: Icenowy Zheng
>
> Signed-off-by: Icenowy Zheng
> ---
> drivers/media/usb/usbtv/usbtv-core.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/media/usb/usbtv/usbtv-core.c
> b/drivers/media/usb/usbtv/usbtv-core.c
> index dc76fd4..0324633 100644
> --- a/drivers/media/usb/usbtv/usbtv-core.c
> +++ b/drivers/media/usb/usbtv/usbtv-core.c
> @@ -141,6 +141,7 @@ static void usbtv_disconnect(struct usb_interface
> *intf)
>
> static struct usb_device_id usbtv_id_table[] = {
> { USB_DEVICE(0x1b71, 0x3002) },
> + { USB_DEVICE(0x1f71, 0x3301) },
> {}
> };
> MODULE_DEVICE_TABLE(usb, usbtv_id_table);
--
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
Re: [PATCH] [media] usbtv: add a new usbid
On Sun, 2016-12-04 at 22:59 +0800, Icenowy Zheng wrote: > > 04.12.2016, 22:00, "Icenowy Zheng" : > > A new usbid of UTV007 is found in a newly bought device. > > > > The usbid is 1f71:3301. > > > > The ID on the chip is: > > UTV007 > > A89029.1 > > 1520L18K1 > > > > Seems that my device come with more capabilities. > > I tested it under Windows, and I got wireless Analog TV > and FM radio functions. (An antenna is shipped with my device) > > Maybe a new radio function is be added, combined with the > new USB ID. > > But at least Composite AV function works well with current usbtv > driver and XawTV. Well, someone with the hardware would need to capture the traffic from the Windows driver (and ideally also extend the driver). Would you mind giving it a try? Do you have a link to some further details about the device you got? Perhaps if it's available cheaply from dealextreme or somewhere I could take a look too. Lubo -- 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
Re: [PATCH] [media] usbtv: don't do DMA on stack
On Wed, 2016-11-16 at 13:15 -0200, Mauro Carvalho Chehab wrote:
> As reported by smatch:
> drivers/media/usb/usbtv/usbtv-video.c:716 usbtv_s_ctrl() error:
> doing dma on the stack (data)
> drivers/media/usb/usbtv/usbtv-video.c:758 usbtv_s_ctrl() error:
> doing dma on the stack (data)
>
> We should not do it, as it won't work on Kernels 4.9 and upper.
> So, alloc a buffer for it.
>
> Fixes: c53a846c48f2 ("[media] usbtv: add video controls")
> Signed-off-by: Mauro Carvalho Chehab
> ---
> drivers/media/usb/usbtv/usbtv-video.c | 18 +-
> 1 file changed, 13 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/media/usb/usbtv/usbtv-video.c
> b/drivers/media/usb/usbtv/usbtv-video.c
> index 86ffbf8780f2..d3b6d3dfaa09 100644
> --- a/drivers/media/usb/usbtv/usbtv-video.c
> +++ b/drivers/media/usb/usbtv/usbtv-video.c
> @@ -704,10 +704,14 @@ static int usbtv_s_ctrl(struct v4l2_ctrl *ctrl)
> {
> struct usbtv *usbtv = container_of(ctrl->handler, struct
> usbtv,
> ctrl
> );
> - u8 data[3];
> + u8 *data;
> u16 index, size;
> int ret;
>
> + data = kmalloc(3, GFP_KERNEL);
> + if (!data)
> + return -ENOMEM;
> +
> /*
> * Read in the current brightness/contrast registers. We
> need them
> * both, because the values are for some reason interleaved.
> @@ -717,6 +721,8 @@ static int usbtv_s_ctrl(struct v4l2_ctrl *ctrl)
> usb_sndctrlpipe(usbtv->udev, 0),
> USBTV_CONTROL_REG,
> USB_DIR_OUT | USB_TYPE_VENDOR |
> USB_RECIP_DEVICE,
> 0, USBTV_BASE + 0x0244, (void *)data, 3, 0);
> + if (ret < 0)
> + goto error;
> }
>
> switch (ctrl->id) {
> @@ -752,6 +758,7 @@ static int usbtv_s_ctrl(struct v4l2_ctrl *ctrl)
> }
> break;
> default:
> + kfree(data);
> return -EINVAL;
> }
>
> @@ -759,12 +766,13 @@ static int usbtv_s_ctrl(struct v4l2_ctrl *ctrl)
> USBTV_CONTROL_REG,
> USB_DIR_OUT | USB_TYPE_VENDOR |
> USB_RECIP_DEVICE,
> 0, index, (void *)data, size, 0);
> - if (ret < 0) {
> +
> +error:
> + if (ret < 0)
> dev_warn(usbtv->dev, "Failed to submit a control
> request.\n");
> - return ret;
> - }
>
> - return 0;
> + kfree(data);
> + return ret;
> }
>
> static const struct v4l2_ctrl_ops usbtv_ctrl_ops = {
Reviewed-by: Lubomir Rintel
Thank you,
Lubo
--
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
[PATCH] [media] usbtv: add video controls
Brightness, Contrast, Hue and Color Saturation are supported.
Signed-off-by: Lubomir Rintel
---
drivers/media/usb/usbtv/usbtv-video.c | 97 ++-
drivers/media/usb/usbtv/usbtv.h | 3 ++
2 files changed, 99 insertions(+), 1 deletion(-)
diff --git a/drivers/media/usb/usbtv/usbtv-video.c
b/drivers/media/usb/usbtv/usbtv-video.c
index 2a08975..dca4fcb 100644
--- a/drivers/media/usb/usbtv/usbtv-video.c
+++ b/drivers/media/usb/usbtv/usbtv-video.c
@@ -1,5 +1,5 @@
/*
- * Copyright (c) 2013 Lubomir Rintel
+ * Copyright (c) 2013,2016 Lubomir Rintel
* All rights reserved.
*
* Redistribution and use in source and binary forms, with or without
@@ -259,6 +259,10 @@ static int usbtv_setup_capture(struct usbtv *usbtv)
if (ret)
return ret;
+ ret = v4l2_ctrl_handler_setup(&usbtv->ctrl);
+ if (ret)
+ return ret;
+
return 0;
}
@@ -696,11 +700,83 @@ static struct vb2_ops usbtv_vb2_ops = {
.stop_streaming = usbtv_stop_streaming,
};
+static int usbtv_s_ctrl(struct v4l2_ctrl *ctrl)
+{
+ struct usbtv *usbtv = container_of(ctrl->handler, struct usbtv,
+ ctrl);
+ u8 data[3];
+ u16 index, size;
+ int ret;
+
+ /*
+* Read in the current brightness/contrast registers. We need them
+* both, because the values are for some reason interleaved.
+*/
+ if (ctrl->id == V4L2_CID_BRIGHTNESS || ctrl->id == V4L2_CID_CONTRAST) {
+ ret = usb_control_msg(usbtv->udev,
+ usb_sndctrlpipe(usbtv->udev, 0), USBTV_CONTROL_REG,
+ USB_DIR_OUT | USB_TYPE_VENDOR | USB_RECIP_DEVICE,
+ 0, USBTV_BASE + 0x0244, (void *)data, 3, 0);
+ }
+
+ switch (ctrl->id) {
+ case V4L2_CID_BRIGHTNESS:
+ index = USBTV_BASE + 0x0244;
+ size = 3;
+ data[0] &= 0xf0;
+ data[0] |= (ctrl->val >> 8) & 0xf;
+ data[2] = ctrl->val & 0xff;
+ break;
+ case V4L2_CID_CONTRAST:
+ index = USBTV_BASE + 0x0244;
+ size = 3;
+ data[0] &= 0x0f;
+ data[0] |= (ctrl->val >> 4) & 0xf0;
+ data[1] = ctrl->val & 0xff;
+ break;
+ case V4L2_CID_SATURATION:
+ index = USBTV_BASE + 0x0242;
+ data[0] = ctrl->val >> 8;
+ data[1] = ctrl->val & 0xff;
+ size = 2;
+ break;
+ case V4L2_CID_HUE:
+ index = USBTV_BASE + 0x0240;
+ size = 2;
+ if (ctrl->val > 0) {
+ data[0] = 0x92 + (ctrl->val >> 8);
+ data[1] = ctrl->val & 0xff;
+ } else {
+ data[0] = 0x82 + (-ctrl->val >> 8);
+ data[1] = -ctrl->val & 0xff;
+ }
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ ret = usb_control_msg(usbtv->udev, usb_sndctrlpipe(usbtv->udev, 0),
+ USBTV_CONTROL_REG,
+ USB_DIR_OUT | USB_TYPE_VENDOR | USB_RECIP_DEVICE,
+ 0, index, (void *)data, size, 0);
+ if (ret < 0) {
+ dev_warn(usbtv->dev, "Failed to submit a control request.\n");
+ return ret;
+ }
+
+ return 0;
+}
+
+static const struct v4l2_ctrl_ops usbtv_ctrl_ops = {
+ .s_ctrl = usbtv_s_ctrl,
+};
+
static void usbtv_release(struct v4l2_device *v4l2_dev)
{
struct usbtv *usbtv = container_of(v4l2_dev, struct usbtv, v4l2_dev);
v4l2_device_unregister(&usbtv->v4l2_dev);
+ v4l2_ctrl_handler_free(&usbtv->ctrl);
vb2_queue_release(&usbtv->vb2q);
kfree(usbtv);
}
@@ -731,7 +807,24 @@ int usbtv_video_init(struct usbtv *usbtv)
return ret;
}
+ /* controls */
+ v4l2_ctrl_handler_init(&usbtv->ctrl, 4);
+ v4l2_ctrl_new_std(&usbtv->ctrl, &usbtv_ctrl_ops,
+ V4L2_CID_CONTRAST, 0, 0x3ff, 1, 0x1d0);
+ v4l2_ctrl_new_std(&usbtv->ctrl, &usbtv_ctrl_ops,
+ V4L2_CID_BRIGHTNESS, 0, 0x3ff, 1, 0x1c0);
+ v4l2_ctrl_new_std(&usbtv->ctrl, &usbtv_ctrl_ops,
+ V4L2_CID_SATURATION, 0, 0x3ff, 1, 0x200);
+ v4l2_ctrl_new_std(&usbtv->ctrl, &usbtv_ctrl_ops,
+ V4L2_CID_HUE, -0xdff, 0xdff, 1, 0x000);
+ ret = usbtv->ctrl.error;
+ if (ret < 0) {
+ dev_warn(usbtv->dev, "Could not initialize controls\n");
+ goto ctrl_fail;
+ }
+
/* v4l2 structure */
+
[PATCH] [media] usbtv: improve a comment
Patrick Keshishian improved the explanation of the protocol when porting
the driver to OpenBSD. Given it's a reverse engineering one and there's
no documetnation it might be helpful to whoever hacks on the driver.
Signed-off-by: Patrick Keshishian
Signed-off-by: Lubomir Rintel
---
drivers/media/usb/usbtv/usbtv-video.c | 19 +--
1 file changed, 17 insertions(+), 2 deletions(-)
diff --git a/drivers/media/usb/usbtv/usbtv-video.c
b/drivers/media/usb/usbtv/usbtv-video.c
index d94d5c5..bae4944 100644
--- a/drivers/media/usb/usbtv/usbtv-video.c
+++ b/drivers/media/usb/usbtv/usbtv-video.c
@@ -265,8 +265,23 @@ static int usbtv_setup_capture(struct usbtv *usbtv)
/* Copy data from chunk into a frame buffer, deinterlacing the data
* into every second line. Unfortunately, they don't align nicely into
* 720 pixel lines, as the chunk is 240 words long, which is 480 pixels.
- * Therefore, we break down the chunk into two halves before copyting,
- * so that we can interleave a line if needed. */
+ * Therefore, we break down the chunk into two halves before copying,
+ * so that we can interleave a line if needed.
+ *
+ * Each "chunk" is 240 words; a word in this context equals 4 bytes.
+ * Image format is YUYV/YUV 4:2:2, consisting of Y Cr Y Cb, defining two
+ * pixels, the Cr and Cb shared between the two pixels, but each having
+ * separate Y values. Thus, the 240 words equal 480 pixels. It therefore,
+ * takes 1.5 chunks to make a 720 pixel-wide line for the frame.
+ * The image is interlaced, so there is a "scan" of odd lines, followed
+ * by "scan" of even numbered lines.
+ *
+ * Following code is writing the chunks in correct sequence, skipping
+ * the rows based on "odd" value.
+ * line 1: chunk[0][ 0..479] chunk[0][480..959] chunk[1][ 0..479]
+ * line 3: chunk[1][480..959] chunk[2][ 0..479] chunk[2][480..959]
+ * ...etc.
+ */
static void usbtv_chunk_to_vbuf(u32 *frame, __be32 *src, int chunk_no, int odd)
{
int half;
--
2.5.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
[PATCH] [media] usbtv: clarify the licensing
OpenBSD would like to reuse some code but consider the licensing not
clear enough. Let's clarify it a bit so that it suits their conventions:
1.) Keep the "extra text" away from the copyright statement and the
rights grant.
2.) Add the warranty disclaimer -- it should not be legally required,
nevertheless the clause 1. of the rights grant refest to it.
Signed-off-by: Lubomir Rintel
Acked-by: Federico Simoncelli
---
drivers/media/usb/usbtv/usbtv-audio.c | 28 ++--
drivers/media/usb/usbtv/usbtv-core.c | 40 +++
drivers/media/usb/usbtv/usbtv-video.c | 40 +++
drivers/media/usb/usbtv/usbtv.h | 22 +++
4 files changed, 93 insertions(+), 37 deletions(-)
diff --git a/drivers/media/usb/usbtv/usbtv-audio.c
b/drivers/media/usb/usbtv/usbtv-audio.c
index 78c12d2..d4b4db3 100644
--- a/drivers/media/usb/usbtv/usbtv-audio.c
+++ b/drivers/media/usb/usbtv/usbtv-audio.c
@@ -1,13 +1,6 @@
/*
- * Fushicai USBTV007 Audio-Video Grabber Driver
- *
- * Product web site:
- *
http://www.fushicai.com/products_detail/&productId=d05449ee-b690-42f9-a661-aa7353894bed.html
- *
* Copyright (c) 2013 Federico Simoncelli
* All rights reserved.
- * No physical hardware was harmed running Windows during the
- * reverse-engineering activity
*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions
@@ -20,6 +13,27 @@
*
* Alternatively, this software may be distributed under the terms of the
* GNU General Public License ("GPL").
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+ * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+ * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+ * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+ * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+ * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+ * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+ * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+/*
+ * Fushicai USBTV007 Audio-Video Grabber Driver
+ *
+ * Product web site:
+ *
http://www.fushicai.com/products_detail/&productId=d05449ee-b690-42f9-a661-aa7353894bed.html
+ *
+ * No physical hardware was harmed running Windows during the
+ * reverse-engineering activity
*/
#include
diff --git a/drivers/media/usb/usbtv/usbtv-core.c
b/drivers/media/usb/usbtv/usbtv-core.c
index 29428be..dc76fd4 100644
--- a/drivers/media/usb/usbtv/usbtv-core.c
+++ b/drivers/media/usb/usbtv/usbtv-core.c
@@ -1,19 +1,6 @@
/*
- * Fushicai USBTV007 Audio-Video Grabber Driver
- *
- * Product web site:
- *
http://www.fushicai.com/products_detail/&productId=d05449ee-b690-42f9-a661-aa7353894bed.html
- *
- * Following LWN articles were very useful in construction of this driver:
- * Video4Linux2 API series: http://lwn.net/Articles/203924/
- * videobuf2 API explanation: http://lwn.net/Articles/447435/
- * Thanks go to Jonathan Corbet for providing this quality documentation.
- * He is awesome.
- *
* Copyright (c) 2013 Lubomir Rintel
* All rights reserved.
- * No physical hardware was harmed running Windows during the
- * reverse-engineering activity
*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions
@@ -26,6 +13,33 @@
*
* Alternatively, this software may be distributed under the terms of the
* GNU General Public License ("GPL").
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+ * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+ * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+ * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+ * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+ * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+ * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+ * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+/*
+ * Fushicai USBTV007 Audio-Video Grabber Driver
+ *
+ * Product web site:
+ *
http://www.fushicai.com/products_detail/&productId=d05449ee-b690-42f9-a661-aa7353894bed.html
+ *
+ * Following LWN articles were very useful in construction of t
Re: [PATCH] usbtv: discard redundant video fields
On Sun, 2015-12-20 at 12:57 +0100, Nikola Forró wrote: > There are many dropped fields with some sources, leading to many > redundant fields without counterparts. When this redundant field > is odd, a new frame is pushed containing this odd field interleaved > with whatever was left in the buffer, causing video artifacts. > > Do not push a new frame after processing every odd field, but do it > only after those which come after an even field. > > Signed-off-by: Nikola Forró Acked-by: Lubomir Rintel Thanks, Lubo -- 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
[RESEND PATCH] saa7146: Create a device name before it's used
request_irq() uses it, tries to create a procfs file with an empty name
otherwise.
Link: https://bugzilla.kernel.org/show_bug.cgi?id=83771
Signed-off-by: Lubomir Rintel
---
drivers/media/common/saa7146/saa7146_core.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/media/common/saa7146/saa7146_core.c
b/drivers/media/common/saa7146/saa7146_core.c
index 97afee6..4418119 100644
--- a/drivers/media/common/saa7146/saa7146_core.c
+++ b/drivers/media/common/saa7146/saa7146_core.c
@@ -364,6 +364,9 @@ static int saa7146_init_one(struct pci_dev *pci, const
struct pci_device_id *ent
goto out;
}
+ /* create a nice device name */
+ sprintf(dev->name, "saa7146 (%d)", saa7146_num);
+
DEB_EE("pci:%p\n", pci);
err = pci_enable_device(pci);
@@ -438,9 +441,6 @@ static int saa7146_init_one(struct pci_dev *pci, const
struct pci_device_id *ent
/* the rest + print status message */
- /* create a nice device name */
- sprintf(dev->name, "saa7146 (%d)", saa7146_num);
-
pr_info("found saa7146 @ mem %p (revision %d, irq %d)
(0x%04x,0x%04x)\n",
dev->mem, dev->revision, pci->irq,
pci->subsystem_vendor, pci->subsystem_device);
--
1.9.3
--
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
[PATCH] saa7146: Create a device name before it's used
request_irq() uses it, tries to create a procfs file with an empty name
otherwise.
Link: https://bugzilla.kernel.org/show_bug.cgi?id=83771
Signed-off-by: Lubomir Rintel
---
drivers/media/common/saa7146/saa7146_core.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/media/common/saa7146/saa7146_core.c
b/drivers/media/common/saa7146/saa7146_core.c
index 97afee6..4418119 100644
--- a/drivers/media/common/saa7146/saa7146_core.c
+++ b/drivers/media/common/saa7146/saa7146_core.c
@@ -364,6 +364,9 @@ static int saa7146_init_one(struct pci_dev *pci, const
struct pci_device_id *ent
goto out;
}
+ /* create a nice device name */
+ sprintf(dev->name, "saa7146 (%d)", saa7146_num);
+
DEB_EE("pci:%p\n", pci);
err = pci_enable_device(pci);
@@ -438,9 +441,6 @@ static int saa7146_init_one(struct pci_dev *pci, const
struct pci_device_id *ent
/* the rest + print status message */
- /* create a nice device name */
- sprintf(dev->name, "saa7146 (%d)", saa7146_num);
-
pr_info("found saa7146 @ mem %p (revision %d, irq %d)
(0x%04x,0x%04x)\n",
dev->mem, dev->revision, pci->irq,
pci->subsystem_vendor, pci->subsystem_device);
--
1.9.3
--
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
Re: [PATCH 2/2] usbtv: add audio support
Hi,
On Tue, 2014-02-04 at 06:22 -0200, Mauro Carvalho Chehab wrote:
> Em Tue, 7 Jan 2014 23:13:22 +0100
> Federico Simoncelli escreveu:
>
> > From: Federico Simoncelli
> >
> > Signed-off-by: Federico Simoncelli
> > Tested-by: Lubomir Rintel
> > ---
> > drivers/media/usb/usbtv/Makefile | 3 +-
> > drivers/media/usb/usbtv/usbtv-audio.c | 384
> > ++
> > drivers/media/usb/usbtv/usbtv-core.c | 16 +-
> > drivers/media/usb/usbtv/usbtv-video.c | 9 +-
> > drivers/media/usb/usbtv/usbtv.h | 21 +-
> > 5 files changed, 423 insertions(+), 10 deletions(-)
> > create mode 100644 drivers/media/usb/usbtv/usbtv-audio.c
> >
> > diff --git a/drivers/media/usb/usbtv/Makefile
> > b/drivers/media/usb/usbtv/Makefile
> > index 775316a..f555cf8 100644
> > --- a/drivers/media/usb/usbtv/Makefile
> > +++ b/drivers/media/usb/usbtv/Makefile
> > @@ -1,4 +1,5 @@
> > usbtv-y := usbtv-core.o \
> > - usbtv-video.o
> > + usbtv-video.o \
> > + usbtv-audio.o
> >
> > obj-$(CONFIG_VIDEO_USBTV) += usbtv.o
> > diff --git a/drivers/media/usb/usbtv/usbtv-audio.c
> > b/drivers/media/usb/usbtv/usbtv-audio.c
> > new file mode 100644
> > index 000..3acc52c
> > --- /dev/null
> > +++ b/drivers/media/usb/usbtv/usbtv-audio.c
> > @@ -0,0 +1,384 @@
> > +/*
> > + * Fushicai USBTV007 Audio-Video Grabber Driver
> > + *
> > + * Product web site:
> > + *
> > http://www.fushicai.com/products_detail/&productId=d05449ee-b690-42f9-a661-aa7353894bed.html
> > + *
> > + * Copyright (c) 2013 Federico Simoncelli
> > + * All rights reserved.
> > + * No physical hardware was harmed running Windows during the
> > + * reverse-engineering activity
> > + *
> > + * Redistribution and use in source and binary forms, with or without
> > + * modification, are permitted provided that the following conditions
> > + * are met:
> > + * 1. Redistributions of source code must retain the above copyright
> > + *notice, this list of conditions, and the following disclaimer,
> > + *without modification.
> > + * 2. The name of the author may not be used to endorse or promote products
> > + *derived from this software without specific prior written permission.
> > + *
> > + * Alternatively, this software may be distributed under the terms of the
> > + * GNU General Public License ("GPL").
> > + */
> > +
> > +#include
> > +#include
> > +#include
> > +#include
> > +
> > +#include "usbtv.h"
> > +
> > +static struct snd_pcm_hardware snd_usbtv_digital_hw = {
> > + .info = SNDRV_PCM_INFO_BATCH |
> > + SNDRV_PCM_INFO_MMAP |
> > + SNDRV_PCM_INFO_INTERLEAVED |
> > + SNDRV_PCM_INFO_BLOCK_TRANSFER |
> > + SNDRV_PCM_INFO_MMAP_VALID,
> > + .formats = SNDRV_PCM_FMTBIT_S16_LE,
> > + .rates = SNDRV_PCM_RATE_CONTINUOUS | SNDRV_PCM_RATE_KNOT,
>
> No, the above is wrong. It should be, instead:
>
> .rates = SNDRV_PCM_RATE_48000,
>
> > + .rate_min = 48000,
> > + .rate_max = 48000,
> > + .channels_min = 2,
> > + .channels_max = 2,
>
> > + .period_bytes_min = 64,
> > + .period_bytes_max = 12544,
>
> The above is likely wrong too, as it seems that you're using a fixed
> number of URBs and a fixed URB size.
>
> An invalid period size can cause bad audio artifacts. Basically, you need
> to estimate/check how many URBs you're receiving per second, and what's
> their size, in order to fill these.
>
> I did such review on one of the drivers, at:
>
> http://git.linuxtv.org/mchehab/experimental.git/shortlog/refs/heads/em28xx
>
> In particular, I suggest you to take a look on those patches:
>
> http://git.linuxtv.org/mchehab/experimental.git/commitdiff/1b3fd2d342667005855deae74200195695433259
>
> http://git.linuxtv.org/mchehab/experimental.git/commitdiff/49677aef90de7834e7bb4b0adf95c3342c2c8668
>
> http://git.linuxtv.org/mchehab/experimental.git/commitdiff/a02b9c238b408f69fc78d528b549b85001df98b8
>
> As it provides a way to dynamically fill it in runtime, showing the
> calculus to estimate those values.
This is a bit different situation, it seems. The hardware, weirdly, uses
bulk transfers for sound data, thus there's essentially no chance of
bandwidth control and the little chance to do anything about the
latency.
The only thing we could possibly tune is the maximum size of the bulk
packet. Is that correct? Would that be
Re: [PATCH] [media] usbtv: fix leak at failure path in usbtv_probe()
On Sat, 2014-05-24 at 00:47 +0400, Alexey Khoroshilov wrote: > Error handling code in usbtv_probe() misses usb_put_dev(). > > Found by Linux Driver Verification project (linuxtesting.org). > > Signed-off-by: Alexey Khoroshilov Acked-by: Lubomir Rintel Thank you! Lubo > --- > drivers/media/usb/usbtv/usbtv-core.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/drivers/media/usb/usbtv/usbtv-core.c > b/drivers/media/usb/usbtv/usbtv-core.c > index 2f87ddfa469f..473fab81b602 100644 > --- a/drivers/media/usb/usbtv/usbtv-core.c > +++ b/drivers/media/usb/usbtv/usbtv-core.c > @@ -91,6 +91,8 @@ static int usbtv_probe(struct usb_interface *intf, > return 0; > > usbtv_video_fail: > + usb_set_intfdata(intf, NULL); > + usb_put_dev(usbtv->udev); > kfree(usbtv); > > return ret; -- 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
[PATCH] [media] usbtv: Add support for PAL video source.
From: Georg Kaindl
Signed-off-by: Georg Kaindl
Tested-by: Lubomir Rintel
---
Hi,
this is a patch sent to me by Georg Kaindl, who uses it with ambi-tv [1]. It
looks fine to me and works well, please review and eventually pull it into the
media tree.
[1] https://github.com/gkaindl/ambi-tv
Thanks!
Lubo
drivers/media/usb/usbtv/usbtv.c | 174 ++-
1 files changed, 136 insertions(+), 38 deletions(-)
diff --git a/drivers/media/usb/usbtv/usbtv.c b/drivers/media/usb/usbtv/usbtv.c
index 8a505a9..6222a4a 100644
--- a/drivers/media/usb/usbtv/usbtv.c
+++ b/drivers/media/usb/usbtv/usbtv.c
@@ -50,13 +50,8 @@
#define USBTV_ISOC_TRANSFERS 16
#define USBTV_ISOC_PACKETS 8
-#define USBTV_WIDTH720
-#define USBTV_HEIGHT 480
-
#define USBTV_CHUNK_SIZE 256
#define USBTV_CHUNK240
-#define USBTV_CHUNKS (USBTV_WIDTH * USBTV_HEIGHT \
- / 4 / USBTV_CHUNK)
/* Chunk header. */
#define USBTV_MAGIC_OK(chunk) ((be32_to_cpu(chunk[0]) & 0xff00) \
@@ -65,6 +60,27 @@
#define USBTV_ODD(chunk) ((be32_to_cpu(chunk[0]) & 0xf000) >> 15)
#define USBTV_CHUNK_NO(chunk) (be32_to_cpu(chunk[0]) & 0x0fff)
+#define USBTV_TV_STD (V4L2_STD_525_60 | V4L2_STD_PAL)
+
+/* parameters for supported TV norms */
+struct usbtv_norm_params {
+ v4l2_std_id norm;
+ int cap_width, cap_height;
+};
+
+static struct usbtv_norm_params norm_params[] = {
+ {
+ .norm = V4L2_STD_525_60,
+ .cap_width = 720,
+ .cap_height = 480,
+ },
+ {
+ .norm = V4L2_STD_PAL,
+ .cap_width = 720,
+ .cap_height = 576,
+ }
+};
+
/* A single videobuf2 frame buffer. */
struct usbtv_buf {
struct vb2_buffer vb;
@@ -94,11 +110,38 @@ struct usbtv {
USBTV_COMPOSITE_INPUT,
USBTV_SVIDEO_INPUT,
} input;
+ v4l2_std_id norm;
+ int width, height;
+ int n_chunks;
int iso_size;
unsigned int sequence;
struct urb *isoc_urbs[USBTV_ISOC_TRANSFERS];
};
+static int usbtv_configure_for_norm(struct usbtv *usbtv, v4l2_std_id norm)
+{
+ int i, ret = 0;
+ struct usbtv_norm_params *params = NULL;
+
+ for (i = 0; i < ARRAY_SIZE(norm_params); i++) {
+ if (norm_params[i].norm & norm) {
+ params = &norm_params[i];
+ break;
+ }
+ }
+
+ if (params) {
+ usbtv->width = params->cap_width;
+ usbtv->height = params->cap_height;
+ usbtv->n_chunks = usbtv->width * usbtv->height
+ / 4 / USBTV_CHUNK;
+ usbtv->norm = params->norm;
+ } else
+ ret = -EINVAL;
+
+ return ret;
+}
+
static int usbtv_set_regs(struct usbtv *usbtv, const u16 regs[][2], int size)
{
int ret;
@@ -158,6 +201,57 @@ static int usbtv_select_input(struct usbtv *usbtv, int
input)
return ret;
}
+static int usbtv_select_norm(struct usbtv *usbtv, v4l2_std_id norm)
+{
+ int ret;
+ static const u16 pal[][2] = {
+ { USBTV_BASE + 0x001a, 0x0068 },
+ { USBTV_BASE + 0x010e, 0x0072 },
+ { USBTV_BASE + 0x010f, 0x00a2 },
+ { USBTV_BASE + 0x0112, 0x00b0 },
+ { USBTV_BASE + 0x0117, 0x0001 },
+ { USBTV_BASE + 0x0118, 0x002c },
+ { USBTV_BASE + 0x012d, 0x0010 },
+ { USBTV_BASE + 0x012f, 0x0020 },
+ { USBTV_BASE + 0x024f, 0x0002 },
+ { USBTV_BASE + 0x0254, 0x0059 },
+ { USBTV_BASE + 0x025a, 0x0016 },
+ { USBTV_BASE + 0x025b, 0x0035 },
+ { USBTV_BASE + 0x0263, 0x0017 },
+ { USBTV_BASE + 0x0266, 0x0016 },
+ { USBTV_BASE + 0x0267, 0x0036 }
+ };
+
+ static const u16 ntsc[][2] = {
+ { USBTV_BASE + 0x001a, 0x0079 },
+ { USBTV_BASE + 0x010e, 0x0068 },
+ { USBTV_BASE + 0x010f, 0x009c },
+ { USBTV_BASE + 0x0112, 0x00f0 },
+ { USBTV_BASE + 0x0117, 0x },
+ { USBTV_BASE + 0x0118, 0x00fc },
+ { USBTV_BASE + 0x012d, 0x0004 },
+ { USBTV_BASE + 0x012f, 0x0008 },
+ { USBTV_BASE + 0x024f, 0x0001 },
+ { USBTV_BASE + 0x0254, 0x005f },
+ { USBTV_BASE + 0x025a, 0x0012 },
+ { USBTV_BASE + 0x025b, 0x0001 },
+ { USBTV_BASE + 0x0263, 0x001c },
+ { USBTV_BASE + 0x0266, 0x0011 },
+ { USBTV_BASE + 0x0267, 0x0005 }
+ };
+
+ ret = usbtv_configure_for_norm(usbtv, norm);
+
+ if (!ret) {
+ if (norm & V4L2_STD_525_60)
+ ret = usbtv_set_regs(u
Re: [PATCH -next] [media] usbtv: remove unused including
On Wed, 2013-07-17 at 10:01 +0800, Wei Yongjun wrote: > From: Wei Yongjun > > Remove including that don't need it. > > Signed-off-by: Wei Yongjun Acked-by: Lubomir Rintel -- 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
[PATCH] [media] usbtv: Add S-Video input support
Alongside already existing Composite input.
Signed-off-by: Lubomir Rintel
Cc: Hans Verkuil
Cc: Mauro Carvalho Chehab
Cc: linux-ker...@vger.kernel.org
Cc: linux-media@vger.kernel.org
---
drivers/media/usb/usbtv/usbtv.c | 99 ---
1 files changed, 82 insertions(+), 17 deletions(-)
diff --git a/drivers/media/usb/usbtv/usbtv.c b/drivers/media/usb/usbtv/usbtv.c
index 9165017..9b250a7 100644
--- a/drivers/media/usb/usbtv/usbtv.c
+++ b/drivers/media/usb/usbtv/usbtv.c
@@ -91,17 +91,78 @@ struct usbtv {
u32 frame_id;
int chunks_done;
+ enum {
+ USBTV_COMPOSITE_INPUT,
+ USBTV_SVIDEO_INPUT,
+ } input;
int iso_size;
unsigned int sequence;
struct urb *isoc_urbs[USBTV_ISOC_TRANSFERS];
};
-static int usbtv_setup_capture(struct usbtv *usbtv)
+static int usbtv_set_regs(struct usbtv *usbtv, const u16 regs[][2], int size)
{
int ret;
int pipe = usb_rcvctrlpipe(usbtv->udev, 0);
int i;
- static const u16 protoregs[][2] = {
+
+ for (i = 0; i < size; i++) {
+ u16 index = regs[i][0];
+ u16 value = regs[i][1];
+
+ ret = usb_control_msg(usbtv->udev, pipe, USBTV_REQUEST_REG,
+ USB_DIR_OUT | USB_TYPE_VENDOR | USB_RECIP_DEVICE,
+ value, index, NULL, 0, 0);
+ if (ret < 0)
+ return ret;
+ }
+
+ return 0;
+}
+
+static int usbtv_select_input(struct usbtv *usbtv, int input)
+{
+ int ret;
+
+ static const u16 composite[][2] = {
+ { USBTV_BASE + 0x0105, 0x0060 },
+ { USBTV_BASE + 0x011f, 0x00f2 },
+ { USBTV_BASE + 0x0127, 0x0060 },
+ { USBTV_BASE + 0x00ae, 0x0010 },
+ { USBTV_BASE + 0x0284, 0x00aa },
+ { USBTV_BASE + 0x0239, 0x0060 },
+ };
+
+ static const u16 svideo[][2] = {
+ { USBTV_BASE + 0x0105, 0x0010 },
+ { USBTV_BASE + 0x011f, 0x00ff },
+ { USBTV_BASE + 0x0127, 0x0060 },
+ { USBTV_BASE + 0x00ae, 0x0030 },
+ { USBTV_BASE + 0x0284, 0x0088 },
+ { USBTV_BASE + 0x0239, 0x0060 },
+ };
+
+ switch (input) {
+ case USBTV_COMPOSITE_INPUT:
+ ret = usbtv_set_regs(usbtv, composite, ARRAY_SIZE(composite));
+ break;
+ case USBTV_SVIDEO_INPUT:
+ ret = usbtv_set_regs(usbtv, svideo, ARRAY_SIZE(svideo));
+ break;
+ default:
+ ret = -EINVAL;
+ }
+
+ if (!ret)
+ usbtv->input = input;
+
+ return ret;
+}
+
+static int usbtv_setup_capture(struct usbtv *usbtv)
+{
+ int ret;
+ static const u16 setup[][2] = {
/* These seem to enable the device. */
{ USBTV_BASE + 0x0008, 0x0001 },
{ USBTV_BASE + 0x01d0, 0x00ff },
@@ -189,16 +250,13 @@ static int usbtv_setup_capture(struct usbtv *usbtv)
{ USBTV_BASE + 0x024f, 0x0002 },
};
- for (i = 0; i < ARRAY_SIZE(protoregs); i++) {
- u16 index = protoregs[i][0];
- u16 value = protoregs[i][1];
+ ret = usbtv_set_regs(usbtv, setup, ARRAY_SIZE(setup));
+ if (ret)
+ return ret;
- ret = usb_control_msg(usbtv->udev, pipe, USBTV_REQUEST_REG,
- USB_DIR_OUT | USB_TYPE_VENDOR | USB_RECIP_DEVICE,
- value, index, NULL, 0, 0);
- if (ret < 0)
- return ret;
- }
+ ret = usbtv_select_input(usbtv, usbtv->input);
+ if (ret)
+ return ret;
return 0;
}
@@ -443,10 +501,17 @@ static int usbtv_querycap(struct file *file, void *priv,
static int usbtv_enum_input(struct file *file, void *priv,
struct v4l2_input *i)
{
- if (i->index > 0)
+ switch (i->index) {
+ case USBTV_COMPOSITE_INPUT:
+ strlcpy(i->name, "Composite", sizeof(i->name));
+ break;
+ case USBTV_SVIDEO_INPUT:
+ strlcpy(i->name, "S-Video", sizeof(i->name));
+ break;
+ default:
return -EINVAL;
+ }
- strlcpy(i->name, "Composite", sizeof(i->name));
i->type = V4L2_INPUT_TYPE_CAMERA;
i->std = V4L2_STD_525_60;
return 0;
@@ -486,15 +551,15 @@ static int usbtv_g_std(struct file *file, void *priv,
v4l2_std_id *norm)
static int usbtv_g_input(struct file *file, void *priv, unsigned int *i)
{
- *i = 0;
+ struct usbtv *usbtv = video_drvdata(file);
+ *i = usbtv->input;
return 0;
}
static int usbtv_s_input(struct file *file, void *priv, unsigned int i)
{
- if (i > 0)
- ret
[PATCH 1/2] [media] usbtv: Fix deinterlacing
The image data is laid out a bit more weirdly and thus needs more work to
properly interlace. What we get from hardware is V4L2_FIELD_ALTERNATE, but
since userspace support for it is practically nonexistent, thus we make
V4L2_FIELD_INTERLACED from it so that it's more easily interpreted.
Signed-off-by: Lubomir Rintel
Cc: Hans Verkuil
Cc: Mauro Carvalho Chehab
Cc: linux-ker...@vger.kernel.org
Cc: linux-media@vger.kernel.org
---
drivers/media/usb/usbtv/usbtv.c | 36 +---
1 files changed, 25 insertions(+), 11 deletions(-)
diff --git a/drivers/media/usb/usbtv/usbtv.c b/drivers/media/usb/usbtv/usbtv.c
index d44fa63..bdb87d7 100644
--- a/drivers/media/usb/usbtv/usbtv.c
+++ b/drivers/media/usb/usbtv/usbtv.c
@@ -57,7 +57,7 @@
#define USBTV_CHUNK_SIZE 256
#define USBTV_CHUNK240
#define USBTV_CHUNKS (USBTV_WIDTH * USBTV_HEIGHT \
- / 2 / USBTV_CHUNK)
+ / 4 / USBTV_CHUNK)
/* Chunk header. */
#define USBTV_MAGIC_OK(chunk) ((be32_to_cpu(chunk[0]) & 0xff00) \
@@ -202,6 +202,26 @@ static int usbtv_setup_capture(struct usbtv *usbtv)
return 0;
}
+/* Copy data from chunk into a frame buffer, deinterlacing the data
+ * into every second line. Unfortunately, they don't align nicely into
+ * 720 pixel lines, as the chunk is 240 words long, which is 480 pixels.
+ * Therefore, we break down the chunk into two halves before copyting,
+ * so that we can interleave a line if needed. */
+static void usbtv_chunk_to_vbuf(u32 *frame, u32 *src, int chunk_no, int odd)
+{
+ int half;
+
+ for (half = 0; half < 2; half++) {
+ int part_no = chunk_no * 2 + half;
+ int line = part_no / 3;
+ int part_index = (line * 2 + !odd) * 3 + (part_no % 3);
+
+ u32 *dst = &frame[part_index * USBTV_CHUNK/2];
+ memcpy(dst, src, USBTV_CHUNK/2 * sizeof(*src));
+ src += USBTV_CHUNK/2;
+ }
+}
+
/* Called for each 256-byte image chunk.
* First word identifies the chunk, followed by 240 words of image
* data and padding. */
@@ -218,11 +238,6 @@ static void usbtv_image_chunk(struct usbtv *usbtv, u32
*chunk)
frame_id = USBTV_FRAME_ID(chunk);
odd = USBTV_ODD(chunk);
chunk_no = USBTV_CHUNK_NO(chunk);
-
- /* Deinterlace. TODO: Use interlaced frame format. */
- chunk_no = (chunk_no - chunk_no % 3) * 2 + chunk_no % 3;
- chunk_no += !odd * 3;
-
if (chunk_no >= USBTV_CHUNKS)
return;
@@ -241,12 +256,11 @@ static void usbtv_image_chunk(struct usbtv *usbtv, u32
*chunk)
buf = list_first_entry(&usbtv->bufs, struct usbtv_buf, list);
frame = vb2_plane_vaddr(&buf->vb, 0);
- /* Copy the chunk. */
- memcpy(&frame[chunk_no * USBTV_CHUNK], &chunk[1],
- USBTV_CHUNK * sizeof(chunk[1]));
+ /* Copy the chunk data. */
+ usbtv_chunk_to_vbuf(frame, &chunk[1], chunk_no, odd);
/* Last chunk in a frame, signalling an end */
- if (usbtv->frame_id && chunk_no == USBTV_CHUNKS-1) {
+ if (odd && chunk_no == USBTV_CHUNKS-1) {
int size = vb2_plane_size(&buf->vb, 0);
buf->vb.v4l2_buf.field = V4L2_FIELD_INTERLACED;
@@ -518,7 +532,7 @@ static int usbtv_queue_setup(struct vb2_queue *vq,
if (*nbuffers == 0)
*nbuffers = 2;
*nplanes = 1;
- sizes[0] = USBTV_CHUNK * USBTV_CHUNKS * sizeof(u32);
+ sizes[0] = USBTV_WIDTH * USBTV_HEIGHT / 2 * sizeof(u32);
return 0;
}
--
1.7.1
--
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
[PATCH 2/2] [media] usbtv: Throw corrupted frames away
Ignore out of order data and mark incomplete buffers as errored.
This gets rid of annoying flicker due to occassional garbage from hardware.
Signed-off-by: Lubomir Rintel
Cc: Hans Verkuil
Cc: Mauro Carvalho Chehab
Cc: linux-ker...@vger.kernel.org
Cc: linux-media@vger.kernel.org
---
drivers/media/usb/usbtv/usbtv.c | 15 +--
1 files changed, 13 insertions(+), 2 deletions(-)
diff --git a/drivers/media/usb/usbtv/usbtv.c b/drivers/media/usb/usbtv/usbtv.c
index bdb87d7..acd649c 100644
--- a/drivers/media/usb/usbtv/usbtv.c
+++ b/drivers/media/usb/usbtv/usbtv.c
@@ -89,6 +89,7 @@ struct usbtv {
/* Number of currently processed frame, useful find
* out when a new one begins. */
u32 frame_id;
+ int chunks_done;
int iso_size;
unsigned int sequence;
@@ -242,8 +243,13 @@ static void usbtv_image_chunk(struct usbtv *usbtv, u32
*chunk)
return;
/* Beginning of a frame. */
- if (chunk_no == 0)
+ if (chunk_no == 0) {
usbtv->frame_id = frame_id;
+ usbtv->chunks_done = 0;
+ }
+
+ if (usbtv->frame_id != frame_id)
+ return;
spin_lock_irqsave(&usbtv->buflock, flags);
if (list_empty(&usbtv->bufs)) {
@@ -258,16 +264,21 @@ static void usbtv_image_chunk(struct usbtv *usbtv, u32
*chunk)
/* Copy the chunk data. */
usbtv_chunk_to_vbuf(frame, &chunk[1], chunk_no, odd);
+ usbtv->chunks_done++;
/* Last chunk in a frame, signalling an end */
if (odd && chunk_no == USBTV_CHUNKS-1) {
int size = vb2_plane_size(&buf->vb, 0);
+ enum vb2_buffer_state state = usbtv->chunks_done ==
+ USBTV_CHUNKS ?
+ VB2_BUF_STATE_DONE :
+ VB2_BUF_STATE_ERROR;
buf->vb.v4l2_buf.field = V4L2_FIELD_INTERLACED;
buf->vb.v4l2_buf.sequence = usbtv->sequence++;
v4l2_get_timestamp(&buf->vb.v4l2_buf.timestamp);
vb2_set_plane_payload(&buf->vb, 0, size);
- vb2_buffer_done(&buf->vb, VB2_BUF_STATE_DONE);
+ vb2_buffer_done(&buf->vb, state);
list_del(&buf->list);
}
--
1.7.1
--
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
[PATCH v4] [media] usbtv: Add driver for Fushicai USBTV007 video frame grabber
Reverse-engineered driver for cheapo video digitizer, made from observations of
Windows XP driver. The protocol is not yet completely understood, so far we
don't provide any controls, only support a single format out of three and don't
support the audio device.
Signed-off-by: Lubomir Rintel
Acked-by: Hans Verkuil
Cc: Mauro Carvalho Chehab
Cc: linux-ker...@vger.kernel.org
Cc: linux-media@vger.kernel.org
---
Changes for v2:
- Fix a typo in comment
- Make prototype register settings static const
- Solve parity calculation weirdness
- Attempt to fix interlacing
- Add timestamp to frames
- [v4l2-compliance] Set pix format priv to 0
- Drop usbtv_*_fmt_vid_cap code duplication
- [v4l2-compliance] Add vidioc_create_bufs
- [v4l2-compliance] Use file handle priorities
- Drop "Driver" from initial dev_info
Changes for v3:
- Utilize ARRAY_SIZE
- Drop queryctrl ioctl
- Reduce default number of buffers to two
- Remove unnecessary locking
- video_set_drvdata() before video_register_device()
Changes for v4:
- Fix default buffer count setting
- Use strlcpy() instead of strncpy()
drivers/media/usb/Kconfig|1 +
drivers/media/usb/Makefile |1 +
drivers/media/usb/usbtv/Kconfig | 10 +
drivers/media/usb/usbtv/Makefile |1 +
drivers/media/usb/usbtv/usbtv.c | 696 ++
5 files changed, 709 insertions(+), 0 deletions(-)
create mode 100644 drivers/media/usb/usbtv/Kconfig
create mode 100644 drivers/media/usb/usbtv/Makefile
create mode 100644 drivers/media/usb/usbtv/usbtv.c
diff --git a/drivers/media/usb/Kconfig b/drivers/media/usb/Kconfig
index 0a7d520..8e10267 100644
--- a/drivers/media/usb/Kconfig
+++ b/drivers/media/usb/Kconfig
@@ -17,6 +17,7 @@ source "drivers/media/usb/zr364xx/Kconfig"
source "drivers/media/usb/stkwebcam/Kconfig"
source "drivers/media/usb/s2255/Kconfig"
source "drivers/media/usb/sn9c102/Kconfig"
+source "drivers/media/usb/usbtv/Kconfig"
endif
if MEDIA_ANALOG_TV_SUPPORT
diff --git a/drivers/media/usb/Makefile b/drivers/media/usb/Makefile
index 7f51d7e..0935f47 100644
--- a/drivers/media/usb/Makefile
+++ b/drivers/media/usb/Makefile
@@ -20,3 +20,4 @@ obj-$(CONFIG_VIDEO_STK1160) += stk1160/
obj-$(CONFIG_VIDEO_CX231XX) += cx231xx/
obj-$(CONFIG_VIDEO_TM6000) += tm6000/
obj-$(CONFIG_VIDEO_EM28XX) += em28xx/
+obj-$(CONFIG_VIDEO_USBTV) += usbtv/
diff --git a/drivers/media/usb/usbtv/Kconfig b/drivers/media/usb/usbtv/Kconfig
new file mode 100644
index 000..8864436
--- /dev/null
+++ b/drivers/media/usb/usbtv/Kconfig
@@ -0,0 +1,10 @@
+config VIDEO_USBTV
+tristate "USBTV007 video capture support"
+depends on VIDEO_DEV
+select VIDEOBUF2_VMALLOC
+
+---help---
+ This is a video4linux2 driver for USBTV007 based video capture
devices.
+
+ To compile this driver as a module, choose M here: the
+ module will be called usbtv
diff --git a/drivers/media/usb/usbtv/Makefile b/drivers/media/usb/usbtv/Makefile
new file mode 100644
index 000..28b872f
--- /dev/null
+++ b/drivers/media/usb/usbtv/Makefile
@@ -0,0 +1 @@
+obj-$(CONFIG_VIDEO_USBTV) += usbtv.o
diff --git a/drivers/media/usb/usbtv/usbtv.c b/drivers/media/usb/usbtv/usbtv.c
new file mode 100644
index 000..bf43f87
--- /dev/null
+++ b/drivers/media/usb/usbtv/usbtv.c
@@ -0,0 +1,696 @@
+/*
+ * Fushicai USBTV007 Video Grabber Driver
+ *
+ * Product web site:
+ *
http://www.fushicai.com/products_detail/&productId=d05449ee-b690-42f9-a661-aa7353894bed.html
+ *
+ * Following LWN articles were very useful in construction of this driver:
+ * Video4Linux2 API series: http://lwn.net/Articles/203924/
+ * videobuf2 API explanation: http://lwn.net/Articles/447435/
+ * Thanks go to Jonathan Corbet for providing this quality documentation.
+ * He is awesome.
+ *
+ * Copyright (c) 2013 Lubomir Rintel
+ * All rights reserved.
+ * No physical hardware was harmed running Windows during the
+ * reverse-engineering activity
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ * 1. Redistributions of source code must retain the above copyright
+ *notice, this list of conditions, and the following disclaimer,
+ *without modification.
+ * 2. The name of the author may not be used to endorse or promote products
+ *derived from this software without specific prior written permission.
+ *
+ * Alternatively, this software may be distributed under the terms of the
+ * GNU General Public License ("GPL").
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#include
+#include
+#include
+#include
+
+/* Hardware. */
+#define USBTV_VIDEO_ENDP 0x81
+#define USBTV_BASE 0xc000
+#define USBTV_REQUEST_REG 12
+
+/* Number of con
Re: [PATCH] [media] usbtv: Add driver for Fushicai USBTV007 video frame grabber
On Wed, 2013-06-12 at 09:49 +0200, Hans Verkuil wrote:
...
> > > > +static int usbtv_queryctrl(struct file *file, void *priv,
> > > > + struct v4l2_queryctrl *ctrl)
> > > > +{
> > > > + return -EINVAL;
> > > > +}
> > >
> > > Drop this ioctl. If it doesn't do anything, then don't specify it.
> >
> > It actually does something; EINVAL here for any ctrl signals there's
> > zero controls.
> >
> > When undefined, ENOTTY that is returned is considered invalid by
> > gstreamer source.
>
> What version of gstreamer are you using? Looking at the gstreamer code it
> seems that it can handle ENOTTY at least since September last year. Not
> handling
> ENOTTY is an application bug (there are other - rare - drivers that do not
> have any controls) and as such I really don't like seeing a workaround like
> this in a driver, especially since this seems like it should be working fine
> with the latest gstreamer.
I was using GStreamer from RHEL6. I retried with Fedora 17 and it worked
fine.
Regards,
Lubo
--
Lubomir Rintel
--
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
[PATCH v3] [media] usbtv: Add driver for Fushicai USBTV007 video frame grabber
Reverse-engineered driver for cheapo video digitizer, made from observations of
Windows XP driver. The protocol is not yet completely understood, so far we
don't provide any controls, only support a single format out of three and don't
support the audio device.
Signed-off-by: Lubomir Rintel
Cc: Mauro Carvalho Chehab
Cc: linux-ker...@vger.kernel.org
Cc: linux-media@vger.kernel.org
---
Changes for v2:
- Fix a typo in comment
- Make prototype register settings static const
- Solve parity calculation weirdness
- Attempt to fix interlacing
- Add timestamp to frames
- [v4l2-compliance] Set pix format priv to 0
- Drop usbtv_*_fmt_vid_cap code duplication
- [v4l2-compliance] Add vidioc_create_bufs
- [v4l2-compliance] Use file handle priorities
- Drop "Driver" from initial dev_info
Changes for v3:
- Utilize ARRAY_SIZE
- Drop queryctrl ioctl
- Reduce default number of buffers to two
- Remove unnecessary locking
- video_set_drvdata() before video_register_device()
diff --git a/drivers/media/usb/Kconfig b/drivers/media/usb/Kconfig
index 0a7d520..8e10267 100644
--- a/drivers/media/usb/Kconfig
+++ b/drivers/media/usb/Kconfig
@@ -17,6 +17,7 @@ source "drivers/media/usb/zr364xx/Kconfig"
source "drivers/media/usb/stkwebcam/Kconfig"
source "drivers/media/usb/s2255/Kconfig"
source "drivers/media/usb/sn9c102/Kconfig"
+source "drivers/media/usb/usbtv/Kconfig"
endif
if MEDIA_ANALOG_TV_SUPPORT
diff --git a/drivers/media/usb/Makefile b/drivers/media/usb/Makefile
index 7f51d7e..0935f47 100644
--- a/drivers/media/usb/Makefile
+++ b/drivers/media/usb/Makefile
@@ -20,3 +20,4 @@ obj-$(CONFIG_VIDEO_STK1160) += stk1160/
obj-$(CONFIG_VIDEO_CX231XX) += cx231xx/
obj-$(CONFIG_VIDEO_TM6000) += tm6000/
obj-$(CONFIG_VIDEO_EM28XX) += em28xx/
+obj-$(CONFIG_VIDEO_USBTV) += usbtv/
diff --git a/drivers/media/usb/usbtv/Kconfig b/drivers/media/usb/usbtv/Kconfig
new file mode 100644
index 000..8864436
--- /dev/null
+++ b/drivers/media/usb/usbtv/Kconfig
@@ -0,0 +1,10 @@
+config VIDEO_USBTV
+tristate "USBTV007 video capture support"
+depends on VIDEO_DEV
+select VIDEOBUF2_VMALLOC
+
+---help---
+ This is a video4linux2 driver for USBTV007 based video capture
devices.
+
+ To compile this driver as a module, choose M here: the
+ module will be called usbtv
diff --git a/drivers/media/usb/usbtv/Makefile b/drivers/media/usb/usbtv/Makefile
new file mode 100644
index 000..28b872f
--- /dev/null
+++ b/drivers/media/usb/usbtv/Makefile
@@ -0,0 +1 @@
+obj-$(CONFIG_VIDEO_USBTV) += usbtv.o
diff --git a/drivers/media/usb/usbtv/usbtv.c b/drivers/media/usb/usbtv/usbtv.c
new file mode 100644
index 000..d44fa63
--- /dev/null
+++ b/drivers/media/usb/usbtv/usbtv.c
@@ -0,0 +1,696 @@
+/*
+ * Fushicai USBTV007 Video Grabber Driver
+ *
+ * Product web site:
+ *
http://www.fushicai.com/products_detail/&productId=d05449ee-b690-42f9-a661-aa7353894bed.html
+ *
+ * Following LWN articles were very useful in construction of this driver:
+ * Video4Linux2 API series: http://lwn.net/Articles/203924/
+ * videobuf2 API explanation: http://lwn.net/Articles/447435/
+ * Thanks go to Jonathan Corbet for providing this quality documentation.
+ * He is awesome.
+ *
+ * Copyright (c) 2013 Lubomir Rintel
+ * All rights reserved.
+ * No physical hardware was harmed running Windows during the
+ * reverse-engineering activity
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ * 1. Redistributions of source code must retain the above copyright
+ *notice, this list of conditions, and the following disclaimer,
+ *without modification.
+ * 2. The name of the author may not be used to endorse or promote products
+ *derived from this software without specific prior written permission.
+ *
+ * Alternatively, this software may be distributed under the terms of the
+ * GNU General Public License ("GPL").
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#include
+#include
+#include
+#include
+
+/* Hardware. */
+#define USBTV_VIDEO_ENDP 0x81
+#define USBTV_BASE 0xc000
+#define USBTV_REQUEST_REG 12
+
+/* Number of concurrent isochronous urbs submitted.
+ * Higher numbers was seen to overly saturate the USB bus. */
+#define USBTV_ISOC_TRANSFERS 16
+#define USBTV_ISOC_PACKETS 8
+
+#define USBTV_WIDTH720
+#define USBTV_HEIGHT 480
+
+#define USBTV_CHUNK_SIZE 256
+#define USBTV_CHUNK240
+#define USBTV_CHUNKS (USBTV_WIDTH * USBTV_HEIGHT \
+ / 2 / USBTV_CHUNK)
+
+/* Chunk header. */
+#define USBTV_MAGIC_OK(chunk) ((be32_to_cpu(chunk[0]) & 0xff00) \
+
[PATCH] [media] usbtv: Add driver for Fushicai USBTV007 video frame grabber
Reverse-engineered driver for cheapo video digitizer, made from observations of
Windows XP driver. The protocol is not yet completely understood, so far we
don't provide any controls, only support a single format out of three and don't
support the audio device.
Signed-off-by: Lubomir Rintel
Cc: Mauro Carvalho Chehab
Cc: linux-ker...@vger.kernel.org
Cc: linux-media@vger.kernel.org
---
Changes for v2:
- Fix a typo in comment
- Make prototype register settings static const
- Solve parity calculation weirdness
- Attempt to fix interlacing
- Add timestamp to frames
- [v4l2-compliance] Set pix format priv to 0
- Drop usbtv_*_fmt_vid_cap code duplication
- [v4l2-compliance] Add vidioc_create_bufs
- [v4l2-compliance] Use file handle priorities
- Drop "Driver" from initial dev_info
drivers/media/usb/Kconfig|1 +
drivers/media/usb/Makefile |1 +
drivers/media/usb/usbtv/Kconfig | 10 +
drivers/media/usb/usbtv/Makefile |1 +
drivers/media/usb/usbtv/usbtv.c | 715 ++
5 files changed, 728 insertions(+), 0 deletions(-)
create mode 100644 drivers/media/usb/usbtv/Kconfig
create mode 100644 drivers/media/usb/usbtv/Makefile
create mode 100644 drivers/media/usb/usbtv/usbtv.c
diff --git a/drivers/media/usb/Kconfig b/drivers/media/usb/Kconfig
index 0a7d520..8e10267 100644
--- a/drivers/media/usb/Kconfig
+++ b/drivers/media/usb/Kconfig
@@ -17,6 +17,7 @@ source "drivers/media/usb/zr364xx/Kconfig"
source "drivers/media/usb/stkwebcam/Kconfig"
source "drivers/media/usb/s2255/Kconfig"
source "drivers/media/usb/sn9c102/Kconfig"
+source "drivers/media/usb/usbtv/Kconfig"
endif
if MEDIA_ANALOG_TV_SUPPORT
diff --git a/drivers/media/usb/Makefile b/drivers/media/usb/Makefile
index 7f51d7e..0935f47 100644
--- a/drivers/media/usb/Makefile
+++ b/drivers/media/usb/Makefile
@@ -20,3 +20,4 @@ obj-$(CONFIG_VIDEO_STK1160) += stk1160/
obj-$(CONFIG_VIDEO_CX231XX) += cx231xx/
obj-$(CONFIG_VIDEO_TM6000) += tm6000/
obj-$(CONFIG_VIDEO_EM28XX) += em28xx/
+obj-$(CONFIG_VIDEO_USBTV) += usbtv/
diff --git a/drivers/media/usb/usbtv/Kconfig b/drivers/media/usb/usbtv/Kconfig
new file mode 100644
index 000..8864436
--- /dev/null
+++ b/drivers/media/usb/usbtv/Kconfig
@@ -0,0 +1,10 @@
+config VIDEO_USBTV
+tristate "USBTV007 video capture support"
+depends on VIDEO_DEV
+select VIDEOBUF2_VMALLOC
+
+---help---
+ This is a video4linux2 driver for USBTV007 based video capture
devices.
+
+ To compile this driver as a module, choose M here: the
+ module will be called usbtv
diff --git a/drivers/media/usb/usbtv/Makefile b/drivers/media/usb/usbtv/Makefile
new file mode 100644
index 000..28b872f
--- /dev/null
+++ b/drivers/media/usb/usbtv/Makefile
@@ -0,0 +1 @@
+obj-$(CONFIG_VIDEO_USBTV) += usbtv.o
diff --git a/drivers/media/usb/usbtv/usbtv.c b/drivers/media/usb/usbtv/usbtv.c
new file mode 100644
index 000..c2a02c2
--- /dev/null
+++ b/drivers/media/usb/usbtv/usbtv.c
@@ -0,0 +1,715 @@
+/*
+ * Fushicai USBTV007 Video Grabber Driver
+ *
+ * Product web site:
+ *
http://www.fushicai.com/products_detail/&productId=d05449ee-b690-42f9-a661-aa7353894bed.html
+ *
+ * Following LWN articles were very useful in construction of this driver:
+ * Video4Linux2 API series: http://lwn.net/Articles/203924/
+ * videobuf2 API explanation: http://lwn.net/Articles/447435/
+ * Thanks go to Jonathan Corbet for providing this quality documentation.
+ * He is awesome.
+ *
+ * Copyright (c) 2013 Lubomir Rintel
+ * All rights reserved.
+ * No physical hardware was harmed running Windows during the
+ * reverse-engineering activity
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ * 1. Redistributions of source code must retain the above copyright
+ *notice, this list of conditions, and the following disclaimer,
+ *without modification.
+ * 2. The name of the author may not be used to endorse or promote products
+ *derived from this software without specific prior written permission.
+ *
+ * Alternatively, this software may be distributed under the terms of the
+ * GNU General Public License ("GPL").
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#include
+#include
+#include
+#include
+
+/* Hardware. */
+#define USBTV_VIDEO_ENDP 0x81
+#define USBTV_BASE 0xc000
+#define USBTV_REQUEST_REG 12
+
+/* Number of concurrent isochronous urbs submitted.
+ * Higher numbers was seen to overly saturate the USB bus. */
+#define USBTV_ISOC_TRANSFERS 16
+#define USBTV_ISOC_PACKETS 8
+
+#define USBTV_WIDTH720
+#define USBTV_HEIGHT 480
+
+#define USBTV_CHUNK_SIZE 256
+#define USBTV_CHUNK240
+#define US
Re: [PATCH] [media] usbtv: Add driver for Fushicai USBTV007 video frame grabber
On Mon, 2013-06-10 at 13:05 +0200, Hans Verkuil wrote:
> > Also, I the hardware uses V4L2_FIELD_ALTERNATE interlacing, but I couldn't
> > make
> > it work,
>
> What didn't work exactly?
Both mplayer and gstream v4l2src displayed only half of an image. Not
sure which combinations of flags did I use anymore.
> > +static int usbtv_queryctrl(struct file *file, void *priv,
> > + struct v4l2_queryctrl *ctrl)
> > +{
> > + return -EINVAL;
> > +}
>
> Drop this ioctl. If it doesn't do anything, then don't specify it.
It actually does something; EINVAL here for any ctrl signals there's
zero controls.
When undefined, ENOTTY that is returned is considered invalid by
gstreamer source.
> It doesn't look too bad :-) You're using all the latest frameworks which is
> excellent.
Thanks for your time. I'll follow up with a new version that aims to
address all the concerns above (apart for the queryctl change, which
would break with gstreamer).
> Can you run the v4l2-compliance tool and post the output of that tool?
Driver Info:
Driver name : usbtv
Card type : usbtv
Bus info : usb-:00:1d.7-5
Driver version: 3.10.0
Capabilities : 0x8501
Video Capture
Read/Write
Streaming
Device Capabilities
Device Caps : 0x0501
Video Capture
Read/Write
Streaming
Compliance test for device /dev/video1 (not using libv4l2):
Required ioctls:
test VIDIOC_QUERYCAP: OK
Allow for multiple opens:
test second video open: OK
test VIDIOC_QUERYCAP: OK
test VIDIOC_G/S_PRIORITY: OK
Debug ioctls:
test VIDIOC_DBG_G/S_REGISTER: OK (Not Supported)
test VIDIOC_LOG_STATUS: OK (Not Supported)
Input ioctls:
test VIDIOC_G/S_TUNER: OK (Not Supported)
test VIDIOC_G/S_FREQUENCY: OK (Not Supported)
test VIDIOC_S_HW_FREQ_SEEK: OK (Not Supported)
test VIDIOC_ENUMAUDIO: OK (Not Supported)
test VIDIOC_G/S/ENUMINPUT: OK
test VIDIOC_G/S_AUDIO: OK (Not Supported)
Inputs: 1 Audio Inputs: 0 Tuners: 0
Output ioctls:
test VIDIOC_G/S_MODULATOR: OK (Not Supported)
test VIDIOC_G/S_FREQUENCY: OK (Not Supported)
test VIDIOC_ENUMAUDOUT: OK (Not Supported)
test VIDIOC_G/S/ENUMOUTPUT: OK (Not Supported)
test VIDIOC_G/S_AUDOUT: OK (Not Supported)
Outputs: 0 Audio Outputs: 0 Modulators: 0
Control ioctls:
test VIDIOC_QUERYCTRL/MENU: OK
test VIDIOC_G/S_CTRL: OK (Not Supported)
test VIDIOC_G/S/TRY_EXT_CTRLS: OK (Not Supported)
test VIDIOC_(UN)SUBSCRIBE_EVENT/DQEVENT: OK (Not Supported)
test VIDIOC_G/S_JPEGCOMP: OK (Not Supported)
Standard Controls: 0 Private Controls: 0
Input/Output configuration ioctls:
test VIDIOC_ENUM/G/S/QUERY_STD: OK
test VIDIOC_ENUM/G/S/QUERY_DV_TIMINGS: OK (Not Supported)
test VIDIOC_DV_TIMINGS_CAP: OK (Not Supported)
Format ioctls:
test VIDIOC_ENUM_FMT/FRAMESIZES/FRAMEINTERVALS: OK
test VIDIOC_G/S_PARM: OK
test VIDIOC_G_FBUF: OK (Not Supported)
test VIDIOC_G_FMT: OK
test VIDIOC_TRY_FMT: OK
test VIDIOC_S_FMT: OK
test VIDIOC_G_SLICED_VBI_CAP: OK (Not Supported)
Codec ioctls:
test VIDIOC_(TRY_)ENCODER_CMD: OK (Not Supported)
test VIDIOC_G_ENC_INDEX: OK (Not Supported)
test VIDIOC_(TRY_)DECODER_CMD: OK (Not Supported)
Buffer ioctls:
test VIDIOC_REQBUFS/CREATE_BUFS/QUERYBUF: OK
Total: 36, Succeeded: 36, Failed: 0, Warnings: 0
--
Lubomir Rintel
--
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
[PATCH] [media] usbtv: Add driver for Fushicai USBTV007 video frame grabber
Reverse-engineered driver for cheapo video digitizer, made from observations of Windows XP driver. The protocol is not yet completely understood, so far we don't provide any controls, only support a single format out of three and don't support the audio device. Signed-off-by: Lubomir Rintel Cc: Mauro Carvalho Chehab Cc: linux-ker...@vger.kernel.org Cc: linux-media@vger.kernel.org --- Hi everyone, this is my first experience with v4l2, videobuf2, usb and a video video hardware. Therefore, please be careful reviewing this as I could have made mistakes that are not likely to happen for more experienced people. I've not figured out the controls for the hardware yet, thus the huge set of unidentified register settings. I've been very unsuccessfully struggling with my Windows installation (the Windows driver for hardware was not usable enough with any other player than the cranky one shipped with it, crashing and deadlocking quite often). It seems quite possible to create a simpler directshow app that would just set the controls; and I plan to do that as time permits. Also, I the hardware uses V4L2_FIELD_ALTERNATE interlacing, but I couldn't make it work, so I'm doing deinterlacing in the driver, which is obviously not the right thing to do. Could anyone educate me on proper way of dealing with data interlaced this way? I could not find a decent example, and I'm not even sure what the sizes in format specification are (like, is the height after or before deinterlacing?). Thanks a lot! Lubo drivers/media/usb/Kconfig|1 + drivers/media/usb/Makefile |1 + drivers/media/usb/usbtv/Kconfig | 10 + drivers/media/usb/usbtv/Makefile |1 + drivers/media/usb/usbtv/usbtv.c | 723 ++ 5 files changed, 736 insertions(+), 0 deletions(-) create mode 100644 drivers/media/usb/usbtv/Kconfig create mode 100644 drivers/media/usb/usbtv/Makefile create mode 100644 drivers/media/usb/usbtv/usbtv.c diff --git a/drivers/media/usb/Kconfig b/drivers/media/usb/Kconfig index 0a7d520..8e10267 100644 --- a/drivers/media/usb/Kconfig +++ b/drivers/media/usb/Kconfig @@ -17,6 +17,7 @@ source "drivers/media/usb/zr364xx/Kconfig" source "drivers/media/usb/stkwebcam/Kconfig" source "drivers/media/usb/s2255/Kconfig" source "drivers/media/usb/sn9c102/Kconfig" +source "drivers/media/usb/usbtv/Kconfig" endif if MEDIA_ANALOG_TV_SUPPORT diff --git a/drivers/media/usb/Makefile b/drivers/media/usb/Makefile index 7f51d7e..0935f47 100644 --- a/drivers/media/usb/Makefile +++ b/drivers/media/usb/Makefile @@ -20,3 +20,4 @@ obj-$(CONFIG_VIDEO_STK1160) += stk1160/ obj-$(CONFIG_VIDEO_CX231XX) += cx231xx/ obj-$(CONFIG_VIDEO_TM6000) += tm6000/ obj-$(CONFIG_VIDEO_EM28XX) += em28xx/ +obj-$(CONFIG_VIDEO_USBTV) += usbtv/ diff --git a/drivers/media/usb/usbtv/Kconfig b/drivers/media/usb/usbtv/Kconfig new file mode 100644 index 000..8864436 --- /dev/null +++ b/drivers/media/usb/usbtv/Kconfig @@ -0,0 +1,10 @@ +config VIDEO_USBTV +tristate "USBTV007 video capture support" +depends on VIDEO_DEV +select VIDEOBUF2_VMALLOC + +---help--- + This is a video4linux2 driver for USBTV007 based video capture devices. + + To compile this driver as a module, choose M here: the + module will be called usbtv diff --git a/drivers/media/usb/usbtv/Makefile b/drivers/media/usb/usbtv/Makefile new file mode 100644 index 000..28b872f --- /dev/null +++ b/drivers/media/usb/usbtv/Makefile @@ -0,0 +1 @@ +obj-$(CONFIG_VIDEO_USBTV) += usbtv.o diff --git a/drivers/media/usb/usbtv/usbtv.c b/drivers/media/usb/usbtv/usbtv.c new file mode 100644 index 000..d165cb1 --- /dev/null +++ b/drivers/media/usb/usbtv/usbtv.c @@ -0,0 +1,723 @@ +/* + * Fushicai USBTV007 Video Grabber Driver + * + * Product web site: + * http://www.fushicai.com/products_detail/&productId=d05449ee-b690-42f9-a661-aa7353894bed.html + * + * Following LWN articles were very useful in construction of this driver: + * Video4Linux2 API series: http://lwn.net/Articles/203924/ + * videobuf2 API explanation: http://lwn.net/Articles/447435/ + * Thanks go to Jonathan Corbet for providing this quality documentation. + * He is awesome. + * + * Copyright (c) 2013 Lubomir Rintel + * All rights reserved. + * No physical hadrware was harmed running Windows during the + * reverse-engineering activity + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions + * are met: + * 1. Redistributions of source code must retain the above copyright + *notice, this list of conditions, and the following disclaimer, + *without modification. + * 2. The name of the author may not be used to endorse or promote products + *derived from this software without specific prior written permission. + * + * Alternat
