Re: [PATCH v3 2/2] media: rc: remove ir-rx51 in favour of generic pwm-ir-tx

[Pali Rohár]

On Friday 13 July 2018 17:38:25 Ivaylo Dimitrov wrote:
> Hi,
> 
> On 13.07.2018 15:22, Sean Young wrote:
> > The ir-rx51 is a pwm-based TX driver specific to the N900. This can be
> > handled entirely by the generic pwm-ir-tx driver.
> > 
> > Note that the suspend code in the ir-rx51 driver is unnecessary, since
> > during transmit, the process is not in interruptable sleep. The process
> > is not put to sleep until the transmit completes.
> > 
> > Compile tested only.
> > 
> 
> I would like to see this being tested on a real HW, however I am on a
> holiday for the next week so won't be able to test till I am back.
> 
> @Pali - do you have n900 with fremantle, upstream kernel and pierogi to test
> pwm-ir-tx on it?

Hi! Currently on my N900 with Maemo Fremantle is 2.6.28 and 3.12
kernels. And 3.12 is a far away from current upstream kernel.

-- 
Pali Rohár
pali.ro...@gmail.com


signature.asc
Description: PGP signature

Re: [PATCH] media: rc: remove ir-rx51 in favour of generic pwm-ir-tx

[Pali Rohár]

On Wednesday 01 November 2017 11:55:33 Sean Young wrote:
> The ir-rx51 is a pwm-based TX driver specific to the n900. This can be
> handled entirely by the generic pwm-ir-tx driver.
> 
> Note that the suspend code in the ir-rx51 driver is unnecessary, since
> during transmit, the current process is not in interruptable sleep. The
> process is not put to sleep until the transmit completes.

Hello, have you tested this patch that IR transmitter is still working
fine on the real Nokia N900 device?

> Cc: Ivaylo Dimitrov <ivo.g.dimitrov...@gmail.com>
> Cc: Pali Rohár <pali.ro...@gmail.com>
> Cc: Pavel Machek <pa...@ucw.cz>
> Cc: Timo Kokkonen <timo.t.kokko...@iki.fi>
> Signed-off-by: Sean Young <s...@mess.org>
> ---
>  arch/arm/boot/dts/omap3-n900.dts |   2 +-
>  drivers/media/rc/Kconfig |  10 --
>  drivers/media/rc/Makefile|   1 -
>  drivers/media/rc/ir-rx51.c   | 316 
> ---
>  4 files changed, 1 insertion(+), 328 deletions(-)
>  delete mode 100644 drivers/media/rc/ir-rx51.c
> 
> diff --git a/arch/arm/boot/dts/omap3-n900.dts 
> b/arch/arm/boot/dts/omap3-n900.dts
> index 4acd32a1c4ef..fccd2b119c0a 100644
> --- a/arch/arm/boot/dts/omap3-n900.dts
> +++ b/arch/arm/boot/dts/omap3-n900.dts
> @@ -152,7 +152,7 @@
>   };
>  
>   ir: n900-ir {
> - compatible = "nokia,n900-ir";
> + compatible = "pwm-ir-tx";
>   pwms = < 0 26316 0>; /* 38000 Hz */
>   };
>  
> diff --git a/drivers/media/rc/Kconfig b/drivers/media/rc/Kconfig
> index 0f863822889e..354ee5224758 100644
> --- a/drivers/media/rc/Kconfig
> +++ b/drivers/media/rc/Kconfig
> @@ -353,16 +353,6 @@ config IR_TTUSBIR
>  To compile this driver as a module, choose M here: the module will
>  be called ttusbir.
>  
> -config IR_RX51
> - tristate "Nokia N900 IR transmitter diode"
> - depends on (OMAP_DM_TIMER && PWM_OMAP_DMTIMER && ARCH_OMAP2PLUS || 
> COMPILE_TEST) && RC_CORE
> - ---help---
> -Say Y or M here if you want to enable support for the IR
> -transmitter diode built in the Nokia N900 (RX51) device.
> -
> -The driver uses omap DM timers for generating the carrier
> -wave and pulses.
> -
>  source "drivers/media/rc/img-ir/Kconfig"
>  
>  config RC_LOOPBACK
> diff --git a/drivers/media/rc/Makefile b/drivers/media/rc/Makefile
> index a1ef86767aef..59b99a2dc2d4 100644
> --- a/drivers/media/rc/Makefile
> +++ b/drivers/media/rc/Makefile
> @@ -25,7 +25,6 @@ obj-$(CONFIG_IR_MESON) += meson-ir.o
>  obj-$(CONFIG_IR_NUVOTON) += nuvoton-cir.o
>  obj-$(CONFIG_IR_ENE) += ene_ir.o
>  obj-$(CONFIG_IR_REDRAT3) += redrat3.o
> -obj-$(CONFIG_IR_RX51) += ir-rx51.o
>  obj-$(CONFIG_IR_SPI) += ir-spi.o
>  obj-$(CONFIG_IR_STREAMZAP) += streamzap.o
>  obj-$(CONFIG_IR_WINBOND_CIR) += winbond-cir.o
> diff --git a/drivers/media/rc/ir-rx51.c b/drivers/media/rc/ir-rx51.c
> deleted file mode 100644
> index 49265f02e772..
> --- a/drivers/media/rc/ir-rx51.c
> +++ /dev/null
> @@ -1,316 +0,0 @@
> -/*
> - *  Copyright (C) 2008 Nokia Corporation
> - *
> - *  Based on lirc_serial.c
> - *
> - *  This program is free software; you can redistribute it and/or modify
> - *  it under the terms of the GNU General Public License as published by
> - *  the Free Software Foundation; either version 2 of the License, or
> - *  (at your option) any later version.
> - *
> - *  This program is distributed in the hope that it will be useful,
> - *  but WITHOUT ANY WARRANTY; without even the implied warranty of
> - *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> - *  GNU General Public License for more details.
> - */
> -#include 
> -#include 
> -#include 
> -#include 
> -#include 
> -#include 
> -#include 
> -
> -#include 
> -#include 
> -
> -#define WBUF_LEN 256
> -
> -struct ir_rx51 {
> - struct rc_dev *rcdev;
> - struct pwm_device *pwm;
> - struct hrtimer timer;
> - struct device*dev;
> - struct ir_rx51_platform_data *pdata;
> - wait_queue_head_t wqueue;
> -
> - unsigned intfreq;   /* carrier frequency */
> - unsigned intduty_cycle; /* carrier duty cycle */
> - int wbuf[WBUF_LEN];
> - int wbuf_index;
> - unsigned long   device_is_open;
> -};
> -
> -static inline void ir_rx51_on(struct ir_rx51 *ir_rx51)
> -{
> - pwm_enable(ir_rx51->pwm);
> -}
> -
> -static inline void ir_rx51_off(struct ir_rx51 *ir_rx51)
> -{
> - pwm_disable(ir_rx51->pwm);
>

Re: [patch] libv4l2: SDL test application

[Pali Rohár]

On Monday 30 October 2017 17:30:53 Hans Verkuil wrote:
> Hi Pavel,
> 
> On 10/28/2017 09:57 PM, Pavel Machek wrote:
> > Add support for simple SDL test application. Allows taking jpeg
> > snapshots, and is meant to run on phone with touchscreen. Not
> > particulary useful on PC with webcam, but should work.
> 
> When I try to build this I get:
> 
> make[3]: Entering directory '/home/hans/work/src/v4l/v4l-utils/contrib/test'
>   CCLD sdlcam
> /usr/bin/ld: sdlcam-sdlcam.o: undefined reference to symbol 
> 'log2@@GLIBC_2.2.5'
> //lib/x86_64-linux-gnu/libm.so.6: error adding symbols: DSO missing from 
> command line
> collect2: error: ld returned 1 exit status
> Makefile:561: recipe for target 'sdlcam' failed
> make[3]: *** [sdlcam] Error 1
> make[3]: Leaving directory '/home/hans/work/src/v4l/v4l-utils/contrib/test'
> Makefile:475: recipe for target 'all-recursive' failed
> make[2]: *** [all-recursive] Error 1
> make[2]: Leaving directory '/home/hans/work/src/v4l/v4l-utils/contrib'
> Makefile:589: recipe for target 'all-recursive' failed
> make[1]: *** [all-recursive] Error 1
> make[1]: Leaving directory '/home/hans/work/src/v4l/v4l-utils'
> Makefile:516: recipe for target 'all' failed
> make: *** [all] Error 2
> 
> I had to add -lm -ldl -lrt to sdlcam_LDFLAGS. Is that correct?

Is not for <> needed
just -lm? log2 should be in mathematical library.

-- 
Pali Rohár
pali.ro...@gmail.com

Re: support autofocus / autogain in libv4l2

[Pali Rohár]

On Tuesday 25 April 2017 14:28:20 Pavel Machek wrote:
> On Tue 2017-04-25 13:30:09, Pali Rohár wrote:
> > On Tuesday 25 April 2017 13:23:30 Pavel Machek wrote:
> > > Hi!
> > > On Tue 2017-04-25 10:08:15, Pali Rohár wrote:
> > > > On Tuesday 25 April 2017 10:05:38 Pavel Machek wrote:
> > > > > > > It would be nice if more than one application could be accessing 
> > > > > > > the
> > > > > > > camera at the same time... (I.e. something graphical running 
> > > > > > > preview
> > > > > > > then using command line tool to grab a picture.) This one is
> > > > > > > definitely not solveable inside a library...
> > > > > > 
> > > > > > Someone once suggested to have something like pulseaudio for V4L.
> > > > > > For such usage, a server would be interesting. Yet, I would code it
> > > > > > in a way that applications using libv4l will talk with such daemon
> > > > > > in a transparent way.
> > > > > 
> > > > > Yes, we need something like pulseaudio for V4L. And yes, we should
> > > > > make it transparent for applications using libv4l.
> > > > 
> > > > IIRC there is already some effort in writing such "video" server which
> > > > would support accessing more application into webcam video, like
> > > > pulseaudio server for accessing more applications to microphone input.
> > > 
> > > Do you have project name / url / something?
> > 
> > Pinos (renamed from PulseVideo)
> > 
> > https://blogs.gnome.org/uraeus/2015/06/30/introducing-pulse-video/
> > https://cgit.freedesktop.org/~wtay/pinos/
> > 
> > But from git history it looks like it is probably dead now...
> 
> Actually, last commit is an hour ago on "work" branch. Seems alive to
> me ;-).

Great! I just (blindly) looked at master branch and it is old...

> Thanks for pointer...
>   Pavel

-- 
Pali Rohár
pali.ro...@gmail.com

Re: support autofocus / autogain in libv4l2

[Pali Rohár]

On Tuesday 25 April 2017 13:23:30 Pavel Machek wrote:
> Hi!
> On Tue 2017-04-25 10:08:15, Pali Rohár wrote:
> > On Tuesday 25 April 2017 10:05:38 Pavel Machek wrote:
> > > > > It would be nice if more than one application could be accessing the
> > > > > camera at the same time... (I.e. something graphical running preview
> > > > > then using command line tool to grab a picture.) This one is
> > > > > definitely not solveable inside a library...
> > > > 
> > > > Someone once suggested to have something like pulseaudio for V4L.
> > > > For such usage, a server would be interesting. Yet, I would code it
> > > > in a way that applications using libv4l will talk with such daemon
> > > > in a transparent way.
> > > 
> > > Yes, we need something like pulseaudio for V4L. And yes, we should
> > > make it transparent for applications using libv4l.
> > 
> > IIRC there is already some effort in writing such "video" server which
> > would support accessing more application into webcam video, like
> > pulseaudio server for accessing more applications to microphone input.
> 
> Do you have project name / url / something?

Pinos (renamed from PulseVideo)

https://blogs.gnome.org/uraeus/2015/06/30/introducing-pulse-video/
https://cgit.freedesktop.org/~wtay/pinos/

But from git history it looks like it is probably dead now...

-- 
Pali Rohár
pali.ro...@gmail.com

Re: support autofocus / autogain in libv4l2

[Pali Rohár]

On Tuesday 25 April 2017 10:05:38 Pavel Machek wrote:
> > > It would be nice if more than one application could be accessing the
> > > camera at the same time... (I.e. something graphical running preview
> > > then using command line tool to grab a picture.) This one is
> > > definitely not solveable inside a library...
> > 
> > Someone once suggested to have something like pulseaudio for V4L.
> > For such usage, a server would be interesting. Yet, I would code it
> > in a way that applications using libv4l will talk with such daemon
> > in a transparent way.
> 
> Yes, we need something like pulseaudio for V4L. And yes, we should
> make it transparent for applications using libv4l.

IIRC there is already some effort in writing such "video" server which
would support accessing more application into webcam video, like
pulseaudio server for accessing more applications to microphone input.

-- 
Pali Rohár
pali.ro...@gmail.com

Re: [RFC 08/13] smiapp-pll: Take existing divisor into account in minimum divisor check

[Pali Rohár]

On Wednesday 15 February 2017 00:05:03 Sakari Ailus wrote:
> Hi Pavel,
> 
> On Tue, Feb 14, 2017 at 02:40:04PM +0100, Pavel Machek wrote:
> > From: Sakari Ailus <sakari.ai...@iki.fi>
> > 
> > Required added multiplier (and divisor) calculation did not take into
> > account the existing divisor when checking the values against the
> > minimum divisor. Do just that.
> > 
> > Signed-off-by: Sakari Ailus <sakari.ai...@iki.fi>
> > Signed-off-by: Ivaylo Dimitrov <ivo.g.dimitrov...@gmail.com>
> > Signed-off-by: Pavel Machek <pa...@ucw.cz>
> 
> I need to understand again why did I write this patch. :-)
> 
> Could you send me the smiapp driver output with debug level messages
> enabled, please?
> 
> I think the problem was with the secondary sensor.
> 

Hi, search for emails and threads:
Message-Id: <1364719448-29894-1-git-send-email-sakari.ai...@iki.fi>
Message-ID: <5728ed34.3060...@gmail.com>

I think I already resent those information again :-)

-- 
Pali Rohár
pali.ro...@gmail.com

Re: [PATCH] devicetree: Add video bus switch

[Pali Rohár]

On Friday 03 February 2017 13:35:08 Pavel Machek wrote:
> N900 contains front and back camera, with a switch between the
> two. This adds support for the switch component, and it is now
> possible to select between front and back cameras during runtime.

IIRC for controlling cameras on N900 there are two GPIOs. Should not you
have both in switch driver?

-- 
Pali Rohár
pali.ro...@gmail.com
--
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 v5] media: Driver for Toshiba et8ek8 5MP sensor

[Pali Rohár]

On Wednesday 14 December 2016 21:12:02 Pavel Machek wrote:
> Hi!
> 
> > On Wednesday 14 December 2016 13:24:51 Pavel Machek wrote:
> > >  
> > > Add driver for et8ek8 sensor, found in Nokia N900 main camera. Can be
> > > used for taking photos in 2.5MP resolution with fcam-dev.
> > > 
> > > Signed-off-by: Ivaylo Dimitrov <ivo.g.dimitrov...@gmail.com>
> > > Signed-off-by: Pavel Machek <pa...@ucw.cz>
> > > 
> > > ---
> > > From v4 I did cleanups to coding style and removed various oddities.
> > > 
> > > Exposure value is now in native units, which simplifies the code.
> > > 
> > > The patch to add device tree bindings was already acked by device tree
> > > people.
> 
> > > + default:
> > > + WARN_ONCE(1, ET8EK8_NAME ": %s: invalid message length.\n",
> > > +   __func__);
> > 
> > dev_warn_once()
> ...
> > > + if (WARN_ONCE(cnt > ET8EK8_MAX_MSG,
> > > +   ET8EK8_NAME ": %s: too many messages.\n", __func__)) {
> > 
> > Maybe replace it with dev_warn_once() too? That condition in WARN_ONCE
> > does not look nice...
> ...
> > > + if (WARN(next->type != ET8EK8_REG_8BIT &&
> > > +  next->type != ET8EK8_REG_16BIT,
> > > +  "Invalid type = %d", next->type)) {
> > dev_warn()
> >
> > > + WARN_ON(sensor->power_count < 0);
> > 
> > Rather some dev_warn()? Do we need stack trace here?
> 
> I don't see what is wrong with WARN(). These are not expected to
> trigger, if they do we'll fix it. If you feel strongly about this,
> feel free to suggest a patch.

One thing is consistency with other parts of code... On all other places
is used dev_warn and on above 4 places WARN. dev_warn automatically adds
device name for easy debugging...

Another thing is that above WARNs do not write why it is warning. It
just write that some condition is not truth...

> > > +static int et8ek8_i2c_reglist_find_write(struct i2c_client *client,
> > > +  struct et8ek8_meta_reglist *meta,
> > > +  u16 type)
> > > +{
> > > + struct et8ek8_reglist *reglist;
> > > +
> > > + reglist = et8ek8_reglist_find_type(meta, type);
> > > + if (!reglist)
> > > + return -EINVAL;
> > > +
> > > + return et8ek8_i2c_write_regs(client, reglist->regs);
> > > +}
> > > +
> > > +static struct et8ek8_reglist **et8ek8_reglist_first(
> > > + struct et8ek8_meta_reglist *meta)
> > > +{
> > > + return >reglist[0].ptr;
> > > +}
> > 
> > Above code looks like re-implementation of linked-list. Does not kernel
> > already provide some?
> 
> Its actually array of pointers as far as I can tell. I don't think any
> helpers would be useful here.

Ok.

> > > + new = et8ek8_gain_table[gain];
> > > +
> > > + /* FIXME: optimise I2C writes! */
> > 
> > Is this FIMXE still valid?
> 
> Probably. Lets optimize it after merge.
> 
> > > + if (sensor->power_count) {
> > > + WARN_ON(1);
> > 
> > Such warning is probably not useful...
> 
> It should not happen, AFAICT. That's why I'd like to know if it does.

I mean: warning should have better description, what happened. Such
warning for somebody who does not see this code is not useful...

> > > +#include "et8ek8_reg.h"
> > > +
> > > +/*
> > > + * Stingray sensor mode settings for Scooby
> > > + */
> > 
> > Are settings for this sensor Stingray enough?
> 
> Seems to work well enough for me. If more modes are needed, we can add
> them later.

Ok.

> > It was me who copied these sensors settings to kernel driver. And I
> > chose only Stingray as this is what was needed for my N900 for
> > testing... Btw, you could add somewhere my and Ivo's Signed-off and
> > copyright state as we both modified et8ek8.c code...
> 
> Normally, people add copyrights when they modify the code. If you want
> to do it now, please send me a patch. (With those warn_ons too, if you
> care, but I think the code is fine as is).

I think sending patch in unified diff format for such change is
overkill. Just place to header it.

-- 
Pali Rohár
pali.ro...@gmail.com
--
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: [PATCHv6] support for AD5820 camera auto-focus coil

[Pali Rohár]

On Monday 08 August 2016 23:41:32 Pavel Machek wrote:
> On Mon 2016-08-08 11:09:56, Sakari Ailus wrote:
> > On Fri, Aug 05, 2016 at 12:26:11PM +0200, Pavel Machek wrote:
> > > This adds support for AD5820 autofocus coil, found for example in
> > > Nokia N900 smartphone.
> > 
> > Thanks, Pavel!
> > 
> > Let's use V4L2_CID_FOCUS_ABSOLUTE, as is in the patch. If we get
> > something better in the future, we'll switch to that then.
> > 
> > I've applied this to ad5820 branch in my tree.
> 
> Thanks. If I understands things correctly, both DTS patch and this
> patch are waiting in your tree, so we should be good to go for 4.9
> (unless some unexpected problems surface)?
> 
> Best regards,
>   Pavel

Was DTS patch merged into 4.9? At least I do not see updated that dts 
file omap3-n900.dts in linus tree...

-- 
Pali Rohár
pali.ro...@gmail.com


signature.asc
Description: This is a digitally signed message part.

Re: [PATCH v5] media: Driver for Toshiba et8ek8 5MP sensor

[Pali Rohár]

Hi! See inlined some my notes.

On Wednesday 14 December 2016 13:24:51 Pavel Machek wrote:
>  
> Add driver for et8ek8 sensor, found in Nokia N900 main camera. Can be
> used for taking photos in 2.5MP resolution with fcam-dev.
> 
> Signed-off-by: Ivaylo Dimitrov 
> Signed-off-by: Pavel Machek 
> 
> ---
> From v4 I did cleanups to coding style and removed various oddities.
> 
> Exposure value is now in native units, which simplifies the code.
> 
> The patch to add device tree bindings was already acked by device tree
> people.
> 
> diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
> index 2669b4b..6d01e15 100644
> --- a/drivers/media/i2c/Kconfig
> +++ b/drivers/media/i2c/Kconfig
> @@ -667,6 +667,7 @@ config VIDEO_S5K5BAF
> camera sensor with an embedded SoC image signal processor.
>  
>  source "drivers/media/i2c/smiapp/Kconfig"
> +source "drivers/media/i2c/et8ek8/Kconfig"
>  
>  config VIDEO_S5C73M3
>   tristate "Samsung S5C73M3 sensor support"
> diff --git a/drivers/media/i2c/Makefile b/drivers/media/i2c/Makefile
> index 92773b2..5bc7bbe 100644
> --- a/drivers/media/i2c/Makefile
> +++ b/drivers/media/i2c/Makefile
> @@ -2,6 +2,7 @@ msp3400-objs  :=  msp3400-driver.o msp3400-kthreads.o
>  obj-$(CONFIG_VIDEO_MSP3400) += msp3400.o
>  
>  obj-$(CONFIG_VIDEO_SMIAPP)   += smiapp/
> +obj-$(CONFIG_VIDEO_ET8EK8)   += et8ek8/
>  obj-$(CONFIG_VIDEO_CX25840) += cx25840/
>  obj-$(CONFIG_VIDEO_M5MOLS)   += m5mols/
>  obj-y+= soc_camera/
> diff --git a/drivers/media/i2c/et8ek8/Kconfig 
> b/drivers/media/i2c/et8ek8/Kconfig
> new file mode 100644
> index 000..1439936
> --- /dev/null
> +++ b/drivers/media/i2c/et8ek8/Kconfig
> @@ -0,0 +1,6 @@
> +config VIDEO_ET8EK8
> + tristate "ET8EK8 camera sensor support"
> + depends on I2C && VIDEO_V4L2 && VIDEO_V4L2_SUBDEV_API
> + ---help---
> +   This is a driver for the Toshiba ET8EK8 5 MP camera sensor.
> +   It is used for example in Nokia N900 (RX-51).
> diff --git a/drivers/media/i2c/et8ek8/Makefile 
> b/drivers/media/i2c/et8ek8/Makefile
> new file mode 100644
> index 000..66d1b7d
> --- /dev/null
> +++ b/drivers/media/i2c/et8ek8/Makefile
> @@ -0,0 +1,2 @@
> +et8ek8-objs  += et8ek8_mode.o et8ek8_driver.o
> +obj-$(CONFIG_VIDEO_ET8EK8)   += et8ek8.o
> diff --git a/drivers/media/i2c/et8ek8/et8ek8_driver.c 
> b/drivers/media/i2c/et8ek8/et8ek8_driver.c
> new file mode 100644
> index 000..4a638f8
> --- /dev/null
> +++ b/drivers/media/i2c/et8ek8/et8ek8_driver.c
> @@ -0,0 +1,1515 @@
> +/*
> + * et8ek8_driver.c
> + *
> + * Copyright (C) 2008 Nokia Corporation
> + *
> + * Contact: Sakari Ailus 
> + *  Tuukka Toivonen 
> + *  Pavel Machek 
> + *
> + * Based on code from Toni Leinonen .
> + *
> + * This driver is based on the Micron MT9T012 camera imager driver
> + * (C) Texas Instruments.
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License
> + * version 2 as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful, but
> + * WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + * General Public License for more details.
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#include "et8ek8_reg.h"
> +
> +#define ET8EK8_NAME  "et8ek8"
> +#define ET8EK8_PRIV_MEM_SIZE 128
> +#define ET8EK8_MAX_MSG   48
> +
> +struct et8ek8_sensor {
> + struct v4l2_subdev subdev;
> + struct media_pad pad;
> + struct v4l2_mbus_framefmt format;
> + struct gpio_desc *reset;
> + struct regulator *vana;
> + struct clk *ext_clk;
> + u32 xclk_freq;
> +
> + u16 version;
> +
> + struct v4l2_ctrl_handler ctrl_handler;
> + struct v4l2_ctrl *exposure;
> + struct v4l2_ctrl *pixel_rate;
> + struct et8ek8_reglist *current_reglist;
> +
> + u8 priv_mem[ET8EK8_PRIV_MEM_SIZE];
> +
> + struct mutex power_lock;
> + int power_count;
> +};
> +
> +#define to_et8ek8_sensor(sd) container_of(sd, struct et8ek8_sensor, subdev)
> +
> +enum et8ek8_versions {
> + ET8EK8_REV_1 = 0x0001,
> + ET8EK8_REV_2,
> +};
> +
> +/*
> + * This table describes what should be written to the sensor register
> + * for each gain value. The gain(index in the table) is in terms of
> + * 0.1EV, i.e. 10 indexes in the table give 2 time more gain [0] in
> + * the *analog gain, [1] in the digital gain
> + *
> + * Analog gain [dB] = 20*log10(regvalue/32); 0x20..0x100
> + */
> +static struct et8ek8_gain {
> + u16 analog;
> + u16 digital;
> +} 

Re: v4.9-rc1: smiapp divides by zero

[Pali Rohár]

On Sunday 23 October 2016 12:22:13 Pavel Machek wrote:
> Hi!
> 
> I tried to update camera code on n900 to v4.9-rc1, and I'm getting
> some divide by zero, that eventually cascades into fcam-dev not
> working.
> 
> mul is zero in my testing, resulting in divide by zero.
> 
> (Note that this is going from my patched camera-v4.8 tree to
> camera-v4.9 tree.)
> 
> Best regards,
>   Pavel

Hi! Ideally look at existing camera patches. I do not know which one is
last, but here are some links:

https://github.com/freemangordon/linux-n900/tree/v4.6-rc4-n900-camera
https://github.com/freemangordon/linux-n900/tree/camera
https://git.kernel.org/cgit/linux/kernel/git/sre/linux-n900.git/log/?h=n900-camera-ivo
https://git.kernel.org/cgit/linux/kernel/git/sre/linux-n900.git/log/?h=n900-camera

> diff --git a/drivers/media/i2c/smiapp-pll.c
> b/drivers/media/i2c/smiapp-pll.c index 5ad1edb..e0a6edd 100644
> --- a/drivers/media/i2c/smiapp-pll.c
> +++ b/drivers/media/i2c/smiapp-pll.c
> @@ -16,6 +16,8 @@
>   * General Public License for more details.
>   */
> 
> +#define DEBUG
> +
>  #include 
>  #include 
>  #include 
> @@ -457,6 +459,10 @@ int smiapp_pll_calculate(struct device *dev,
>   i = gcd(pll->pll_op_clk_freq_hz, pll->ext_clk_freq_hz);
>   mul = div_u64(pll->pll_op_clk_freq_hz, i);
>   div = pll->ext_clk_freq_hz / i;
> + if (!mul) {
> + dev_err(dev, "forcing mul to 1\n");
> + mul = 1;
> + }
>   dev_dbg(dev, "mul %u / div %u\n", mul, div);
> 
>   min_pre_pll_clk_div =

Is not this patch still enough?
https://patchwork.kernel.org/patch/8921761/

-- 
Pali Rohár
pali.ro...@gmail.com


signature.asc
Description: This is a digitally signed message part.

Re: [PATCHv6] support for AD5820 camera auto-focus coil

[Pali Rohár]

On Monday 08 August 2016 11:09:56 Sakari Ailus wrote:
> On Fri, Aug 05, 2016 at 12:26:11PM +0200, Pavel Machek wrote:
> > 
> > This adds support for AD5820 autofocus coil, found for example in
> > Nokia N900 smartphone.
> 
> Thanks, Pavel!
> 
> Let's use V4L2_CID_FOCUS_ABSOLUTE, as is in the patch. If we get something
> better in the future, we'll switch to that then.

Ok, and what with AD5820_RAMP_TIME and AD5820_RAMP_MODE?

> I've applied this to ad5820 branch in my tree.

Are you going to finally send this patch to media tree or pull request
to Linus?

-- 
Pali Rohár
pali.ro...@gmail.com
--
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: [PATCHv6] support for AD5820 camera auto-focus coil

[Pali Rohár]

On Friday 05 August 2016 12:26:11 Pavel Machek wrote:
> 
> This adds support for AD5820 autofocus coil, found for example in
> Nokia N900 smartphone.
> 
> Signed-off-by: Pavel Machek <pa...@ucw.cz>

Acked-by: Pali Rohár <pali.ro...@gmail.com>

> ---
> v2: simple cleanups, fix error paths, simplify probe
> v3: more cleanups, remove printk, add include
> v4: remove header file.
> v5: switch to devm_ , style cleanups, fixes
> v6: remove new userspace APIs.
> 
> Can we finally get the patch in, please?

This patch is on ML for months or years, right?
Please merge it...

-- 
Pali Rohár
pali.ro...@gmail.com
--
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: [RESEND PATCH v2 0/5] ir-rx51 driver fixes

[Pali Rohár]

On Wednesday 22 June 2016 21:22:16 Ivaylo Dimitrov wrote:
> ir-rx51 is a driver for Nokia N900 IR transmitter. The current series
> fixes the remaining problems in the driver:
> 
>  - replace GP timer 9 with PWM framework usage
>  - replace pulse width timer dmtimer usage with hrtimer
>  - add DT support to the driver
>  - add driver to the board DTS
> 
> Patch 2 is needed so the driver to function correctly, without it PWM
> refuses to set the needed carrier frequency.
> 
> Changes compared to v1:
>  - removed [PATCH 5/7] ARM: OMAP: dmtimer: Do not call PM runtime
>functions when not needed.
>  - DT compatible string changed to "nokia,n900-ir"
> 
> Ivaylo Dimitrov (5):
>   ir-rx51: Fix build after multiarch changes broke it
>   pwm: omap-dmtimer: Allow for setting dmtimer clock source
>   ir-rx51: use PWM framework instead of OMAP dmtimer
>   ir-rx51: add DT support to driver
>   ir-rx51: use hrtimer instead of dmtimer
> 
>  .../devicetree/bindings/media/nokia,n900-ir|  20 ++
>  .../devicetree/bindings/pwm/pwm-omap-dmtimer.txt   |   4 +
>  arch/arm/mach-omap2/board-rx51-peripherals.c   |   5 -
>  arch/arm/mach-omap2/pdata-quirks.c |  10 +-
>  drivers/media/rc/Kconfig   |   2 +-
>  drivers/media/rc/ir-rx51.c | 229
> +++-- drivers/pwm/pwm-omap-dmtimer.c
> |  12 +- include/linux/platform_data/media/ir-rx51.h|  
> 3 -
>  8 files changed, 111 insertions(+), 174 deletions(-)
>  create mode 100644
> Documentation/devicetree/bindings/media/nokia,n900-ir

Looks good, you can add my Acked-by.

-- 
Pali Rohár
pali.ro...@gmail.com


signature.asc
Description: This is a digitally signed message part.

Re: [RESEND PATCH v2 1/5] ir-rx51: Fix build after multiarch changes broke it

[Pali Rohár]

On Wednesday 22 June 2016 21:22:17 Ivaylo Dimitrov wrote:
> The ir-rx51 driver for n900 has been disabled since the multiarch
> changes as plat include directory no longer is SoC specific.
> 
> Let's fix it with minimal changes to pass the dmtimer calls in
> pdata. Then the following changes can be done while things can
> be tested to be working for each change:
> 
> 1. Change the non-pwm dmtimer to use just hrtimer if possible
> 
> 2. Change the pwm dmtimer to use Linux PWM API with the new
>drivers/pwm/pwm-omap-dmtimer.c and remove the direct calls
>to dmtimer functions
> 
> 3. Parse configuration from device tree and drop the pdata
> 
> Note compilation of this depends on the previous patch
> "ARM: OMAP2+: Add more functions to pwm pdata for ir-rx51".

I think that this extensive description is not needed for commit 
message. Just for email discussion.

> Cc: Mauro Carvalho Chehab <mche...@osg.samsung.com>
> Cc: Neil Armstrong <narmstr...@baylibre.com>
> Cc: linux-media@vger.kernel.org
> Signed-off-by: Tony Lindgren <t...@atomide.com>
> Signed-off-by: Ivaylo Dimitrov <ivo.g.dimitrov...@gmail.com>
> Acked-by: Pavel Machek <pa...@ucw.cz>
> ---

-- 
Pali Rohár
pali.ro...@gmail.com


signature.asc
Description: This is a digitally signed message part.

Re: [PATCH v3 1/2] media: Driver for Toshiba et8ek8 5MP sensor

[Pali Rohár]

On Saturday 18 June 2016 18:04:23 Pavel Machek wrote:
> Hi!
> 
> > > > +   .reglist = {
> > > > +   { .ptr = _poweron_mode2_16vga_2592x1968_12_07fps 
> > > > },
> > > > +   { .ptr = _16vga_2592x1968_13_12fps_dpcm10_8 },
> > > > +   { .ptr = _4vga_1296x984_29_99fps_dpcm10_8 },
> > > > +   { .ptr = _svga_864x656_29_88fps },
> > > > +   { .ptr = _vga_648x492_29_93fps },
> > > > +   { .ptr = _16vga_2592x1968_3_99fps },
> > > > +   { .ptr = _648x492_5fps },
> > > > +   { .ptr = _4vga_1296x984_5fps },
> > > > +   { .ptr = _4vga_1296x984_25fps_dpcm10_8 },
> > > > +   { .ptr = 0 }
> > > > +   }
> > > > +};
> > > 
> > > I'd say .ptr = NULL.
> > 
> > Anyway, this code was generated from configuration ini files and
> > perl script available from:
> > https://gitorious.org/omap3camera/camera-firmware
> > 
> > Originally in Maemo above C structure is compiled into binary file
> > and via request_firmware() loaded from userspace to kernel driver.
> > 
> > For me this sounds like a big overkill, so I included above reglist
> > code direcly into et8ek8 kernel driver to avoid request_firmware()
> > and separate userspace storage...
> 
> Yep, that makes sense, thanks for explanation. I guess that means
> that we should put a comment on top of the file explaining what is
> going on.
> 
> Best regards,
>   Pavel

Here is that original stingraytsb_v14_simple.ini file:

https://gitorious.org/omap3camera/camera-firmware?p=omap3camera:camera-firmware.git;a=tree;f=ini

Looks like are are some "non simple" stingraytsb_v14 files too, but I
have no idea which modes defines...

Sakari, any idea?
Which we should include into kernel et8ek8 kernel driver?

-- 
Pali Rohár
pali.ro...@gmail.com


signature.asc
Description: This is a digitally signed message part.

Re: [PATCH v3 1/2] media: Driver for Toshiba et8ek8 5MP sensor

[Pali Rohár]

On Saturday 18 June 2016 17:22:59 Pavel Machek wrote:
> > +/*
> > + *
> > + * Stingray sensor mode settings for Scooby
> > + *
> > + *
> > + */
> 
> I'd fix it to normal comment style... and possibly remove it. Can you
> understand what it says?
> 
> > +   },
> > +   .regs = {
> > +   { ET8EK8_REG_8BIT, 0x1239, 0x4F },  /**/
> > +   { ET8EK8_REG_8BIT, 0x1238, 0x02 },  /**/
> > +   { ET8EK8_REG_8BIT, 0x123B, 0x70 },  /**/
> > +   { ET8EK8_REG_8BIT, 0x123A, 0x05 },  /**/
> > +   { ET8EK8_REG_8BIT, 0x121B, 0x63 },  /**/
> > +   { ET8EK8_REG_8BIT, 0x1220, 0x85 },  /**/
> > +   { ET8EK8_REG_8BIT, 0x1221, 0x00 },  /**/
> > +   { ET8EK8_REG_8BIT, 0x1222, 0x58 },  /**/
> > +   { ET8EK8_REG_8BIT, 0x1223, 0x00 },  /**/
> > +   { ET8EK8_REG_8BIT, 0x121D, 0x63 },  /**/
> > +   { ET8EK8_REG_8BIT, 0x125D, 0x83 },  /**/
> > +   { ET8EK8_REG_TERM, 0, 0}
> > +   }
> 
> I'd remove the empty comments...
> 
> > +struct et8ek8_meta_reglist meta_reglist = {
> > +   .version = "V14 03-June-2008",
> 
> Do we need the version?
> 
> > +   .reglist = {
> > +   { .ptr = _poweron_mode2_16vga_2592x1968_12_07fps },
> > +   { .ptr = _16vga_2592x1968_13_12fps_dpcm10_8 },
> > +   { .ptr = _4vga_1296x984_29_99fps_dpcm10_8 },
> > +   { .ptr = _svga_864x656_29_88fps },
> > +   { .ptr = _vga_648x492_29_93fps },
> > +   { .ptr = _16vga_2592x1968_3_99fps },
> > +   { .ptr = _648x492_5fps },
> > +   { .ptr = _4vga_1296x984_5fps },
> > +   { .ptr = _4vga_1296x984_25fps_dpcm10_8 },
> > +   { .ptr = 0 }
> > +   }
> > +};
> 
> I'd say .ptr = NULL.
> 

Anyway, this code was generated from configuration ini files and perl 
script available from: https://gitorious.org/omap3camera/camera-firmware

Originally in Maemo above C structure is compiled into binary file and 
via request_firmware() loaded from userspace to kernel driver.

For me this sounds like a big overkill, so I included above reglist code 
direcly into et8ek8 kernel driver to avoid request_firmware() and 
separate userspace storage...

And for smia-sensor (front webcam) in that gitorious repository is also 
reglist structure. It is not needed? Can somebody investigate why it is 
not needed?

-- 
Pali Rohár
pali.ro...@gmail.com


signature.asc
Description: This is a digitally signed message part.

Re: [PATCH 0/7] ir-rx51 driver fixes

[Pali Rohár]

On Saturday 07 May 2016 17:21:41 Ivaylo Dimitrov wrote:
> ir-rx51 is a driver for Nokia N900 IR transmitter. The current series
> fixes the remaining problems in the driver:
> 
>  - replace GP timer 9 with PWM framework usage
>  - replace pulse width timer dmtimer usage with hrtimer
>  - add DT support to the driver
>  - add driver to the board DTS
> 
> Pathes 2 and 5 are needed so the driver to function correctly,
> without those PWM either refuses to set the needed carrier frequency
> (patch 2) or there are such a delays in the PWM framework, code that
> transmission duration raises to ~5s instead of half a second.
> 
> Ivaylo Dimitrov (6):
>   pwm: omap-dmtimer: Allow for setting dmtimer clock source
>   [media] ir-rx51: use PWM framework instead of OMAP dmtimer
>   [media] ir-rx51: add DT support to driver
>   ARM: OMAP: dmtimer: Do not call PM runtime functions when not
> needed. [media] ir-rx51: use hrtimer instead of dmtimer
>   ARM: dts: n900: enable lirc-rx51 driver
> 
> Tony Lindgren (1):
>   ir-rx51: Fix build after multiarch changes broke it
> 
>  .../devicetree/bindings/media/nokia,lirc-rx51  |  19 ++
>  .../devicetree/bindings/pwm/pwm-omap-dmtimer.txt   |   4 +
>  arch/arm/boot/dts/omap3-n900.dts   |  12 ++
>  arch/arm/mach-omap2/board-rx51-peripherals.c   |   5 -
>  arch/arm/mach-omap2/pdata-quirks.c |  10 +-
>  arch/arm/plat-omap/dmtimer.c   |   9 +-
>  arch/arm/plat-omap/include/plat/dmtimer.h  |   1 +
>  drivers/media/rc/Kconfig   |   2 +-
>  drivers/media/rc/ir-rx51.c | 229
> +++-- drivers/pwm/pwm-omap-dmtimer.c
> |  12 +- include/linux/platform_data/media/ir-rx51.h|  
> 3 -
>  11 files changed, 131 insertions(+), 175 deletions(-)
>  create mode 100644
> Documentation/devicetree/bindings/media/nokia,lirc-rx51

Patch series looks good, you can add my Acked-by.

-- 
Pali Rohár
pali.ro...@gmail.com


signature.asc
Description: This is a digitally signed message part.

Re: [RFC PATCH 04/24] smiapp-pll: Take existing divisor into account in minimum divisor check

[Pali Rohár]

On Sunday 01 May 2016 13:45:24 Sakari Ailus wrote:
> Hi Ivaylo,
> 
> On Mon, Apr 25, 2016 at 12:08:04AM +0300, Ivaylo Dimitrov wrote:
> > From: Sakari Ailus <sakari.ai...@iki.fi>
> > 
> > Required added multiplier (and divisor) calculation did not take into
> > account the existing divisor when checking the values against the minimum
> > divisor. Do just that.
> > 
> > Signed-off-by: Sakari Ailus <sakari.ai...@iki.fi>
> > ---
> >  drivers/media/i2c/smiapp-pll.c | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/media/i2c/smiapp-pll.c b/drivers/media/i2c/smiapp-pll.c
> > index e3348db..5ad1edb 100644
> > --- a/drivers/media/i2c/smiapp-pll.c
> > +++ b/drivers/media/i2c/smiapp-pll.c
> > @@ -227,7 +227,8 @@ static int __smiapp_pll_calculate(
> >  
> > more_mul_factor = lcm(div, pll->pre_pll_clk_div) / div;
> > dev_dbg(dev, "more_mul_factor: %u\n", more_mul_factor);
> > -   more_mul_factor = lcm(more_mul_factor, op_limits->min_sys_clk_div);
> > +   more_mul_factor = lcm(more_mul_factor,
> > + DIV_ROUND_UP(op_limits->min_sys_clk_div, div));
> > dev_dbg(dev, "more_mul_factor: min_op_sys_clk_div: %d\n",
> > more_mul_factor);
> > i = roundup(more_mul_min, more_mul_factor);
> 
> I remember writing the patch, but I don't remember what for, or whether it
> was really needed. Does the secondary sensor work without this one?

Hi! You sent me this patch more then 3 years ago. Look at our private
email discussion, e.g. email with Message-Id <201303281524.10538@pali>
and subject "Re: Nokia N900 - smiapp driver" which was sent years ago
Thu, 28 Mar 2013 15:24:10 +0100.

-- 
Pali Rohár
pali.ro...@gmail.com
--
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: [PATCHv3] support for AD5820 camera auto-focus coil

[Pali Rohár]

On Saturday 21 May 2016 14:43:43 Ivaylo Dimitrov wrote:
> >diff --git a/include/media/ad5820.h b/include/media/ad5820.h
> >new file mode 100644
> >index 000..f5a1565
> >--- /dev/null
> >+++ b/include/media/ad5820.h
> >@@ -0,0 +1,70 @@
> >+/*
> >+ * include/media/ad5820.h
> >+ *
> >+ * Copyright (C) 2008 Nokia Corporation
> >+ * Copyright (C) 2007 Texas Instruments
> >+ *
> >+ * Contact: Tuukka Toivonen <tuukka.o.toivo...@nokia.com>
> >+ *  Sakari Ailus <sakari.ai...@nokia.com>
> >+ *
> >+ * Based on af_d88.c by Texas Instruments.
> >+ *
> >+ * This program is free software; you can redistribute it and/or
> >+ * modify it under the terms of the GNU General Public License
> >+ * version 2 as published by the Free Software Foundation.
> >+ *
> >+ * This program is distributed in the hope that it will be useful, but
> >+ * WITHOUT ANY WARRANTY; without even the implied warranty of
> >+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> >+ * General Public License for more details.
> >+ *
> >+ * You should have received a copy of the GNU General Public License
> >+ * along with this program; if not, write to the Free Software
> >+ * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA
> >+ * 02110-1301 USA
> >+ */
> >+
> >+#ifndef AD5820_H
> >+#define AD5820_H
> >+
> >+#include 
> >+#include 
> >+#include 
> >+
> >+#include 
> >+#include 
> >+
> >+struct regulator;
> >+
> >+#define AD5820_NAME "ad5820"
> >+#define AD5820_I2C_ADDR (0x18 >> 1)

Maybe write I2C address is more readable form? What is reason such
bit shift format?

> >+/* Register definitions */
> >+#define AD5820_POWER_DOWN   (1 << 15)
> >+#define AD5820_DAC_SHIFT4
> 
> Do those defines really belong here? Isn't it better if they are moved to
> ad5820.c?

For me it looks like this is private for ad5820.c.

> >+#define AD5820_RAMP_MODE_LINEAR (0 << 3)
> >+#define AD5820_RAMP_MODE_64_16  (1 << 3)
> >+
> >+struct ad5820_platform_data {
> >+int (*set_xshutdown)(struct v4l2_subdev *subdev, int set);
> >+};

This is for legacy board code support right? We need DT support for N900
as legacy board code is going to be deleted.

> >+#define to_ad5820_device(sd)container_of(sd, struct ad5820_device, 
> >subdev)
> >+
> >+struct ad5820_device {
> >+struct v4l2_subdev subdev;
> >+struct ad5820_platform_data *platform_data;
> >+    struct regulator *vana;
> >+
> >+struct v4l2_ctrl_handler ctrls;
> >+u32 focus_absolute;
> >+u32 focus_ramp_time;
> >+u32 focus_ramp_mode;
> >+
> >+struct mutex power_lock;
> >+int power_count;
> >+
> >+int standby : 1;
> >+};
> >+
> 
> The same for struct ad5820_device, is it really part of the public API?

Yes, this is also private for ad5820.c

> >+#endif /* AD5820_H */
> >
> >
> >

-- 
Pali Rohár
pali.ro...@gmail.com
--
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: camera application for testing (was Re: v4l subdevs without big device)

[Pali Rohár]

On Saturday 30 April 2016 00:13:59 Pavel Machek wrote:
> Any other application I should look at? Thanks,

Maybe camera-ui, which is part of CSSU?

https://github.com/community-ssu/camera-ui

-- 
Pali Rohár
pali.ro...@gmail.com


signature.asc
Description: This is a digitally signed message part.

Re: v4l subdevs without big device was Re: drivers/media/i2c/adp1653.c: does not show as /dev/video* or v4l-subdev*

[Pali Rohár]

On Friday 29 April 2016 13:59:44 Sakari Ailus wrote:
> > pavel@amd:/data/l/linux-n900$ git fetch
> > git://git.retiisi.org.uk/~sailus/linux.git leds-as3645a:leds-as3645a
> > fatal: unable to connect to git.retiisi.org.uk:
> > git.retiisi.org.uk: Name or service not known
> > 
> > pavel@amd:/data/l/linux-n900$ git fetch
> > git://salottisipuli.retiisi.org.uk/~sailus/linux.git
> > leds-as3645a:leds-as3645a
> > remote: Counting objects: 132, done.
> > remote: Compressing objects: 100% (46/46), done.
> > remote: Total 132 (delta 111), reused 107 (delta 86)
> > Receiving objects: 100% (132/132), 22.80 KiB | 0 bytes/s, done.
> > Resolving deltas: 100% (111/111), completed with 34 local objects.
> > From git://salottisipuli.retiisi.org.uk/~sailus/linux
> >  * [new branch]  leds-as3645a -> leds-as3645a
> 
> Yeah, that works, too. git alias has been added some three weeks ago so
> there seem to be something strange going on with DNS.

Maybe update SOA record?

-- 
Pali Rohár
pali.ro...@gmail.com
--
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: [RFC PATCH 00/24] Make Nokia N900 cameras working

[Pali Rohár]

On Monday 25 April 2016 16:14:41 Pali Rohár wrote:
> > Anyway, does anyone know where to get the media-ctl tool?
> 
> Looks like it is part of v4l-utils package. At least in git:
> https://git.linuxtv.org/v4l-utils.git/tree/utils/media-ctl
> 
> > It does not seem to be in debian 7 or debian 8...
> 
> I do not see it in debian too, but there is some version in ubuntu:
> http://packages.ubuntu.com/trusty/media-ctl
> 
> So you can compile ubuntu dsc package, should work on debian.

Finally, it is also in debian, see:

https://packages.debian.org/search?suite=sid=any=path=contents=media-ctl
https://packages.debian.org/sid/amd64/v4l-utils/filelist

-- 
Pali Rohár
pali.ro...@gmail.com


signature.asc
Description: This is a digitally signed message part.

Re: [RFC PATCH 00/24] Make Nokia N900 cameras working

[Pali Rohár]

On Monday 25 April 2016 16:06:12 Pavel Machek wrote:
> Hi!
> 
> > On Monday 25 April 2016 00:08:00 Ivaylo Dimitrov wrote:
> > > The needed pipeline could be made with:
> > > 
> > > media-ctl -r
> > > media-ctl -l '"vs6555 binner 2-0010":1 -> "video-bus-switch":2
> ...
> > On Monday 25 April 2016 09:33:18 Ivaylo Dimitrov wrote:
> > > Try with:
> > > 
> > > media-ctl -r
> > > media-ctl -l '"et8ek8 3-003e":0 -> "video-bus-switch":1 [1]'
> ...
> > > mplayer -tv 
> > > driver=v4l2:width=800:height=600:outfmt=uyvy:device=/dev/video6 -vo xv 
> > > -vf screenshot tv://
> > 
> > Hey!!! That is crazy! Who created such retard API?? In both cases you
> > are going to show video from /dev/video6 device. But in real I have two
> > independent camera devices: front and back.
> 
> Because Nokia, and because the hardware is complex, I'm afraid.

In Nokia kernel, there are just /dev/video0 and /dev/video1. When I open
first I see back camera, second front camera. No media-ctl nor any other
reconfiguration is needed. So not Nokia nor hw complexity is reason...

> First we need to get it to work, than we can improve v4l... 

Ok, I agree. But I really would like to see just two video devices and
all those route configuration in kernel...

> Anyway, does anyone know where to get the media-ctl tool?

Looks like it is part of v4l-utils package. At least in git:
https://git.linuxtv.org/v4l-utils.git/tree/utils/media-ctl

> It does not seem to be in debian 7 or debian 8...

I do not see it in debian too, but there is some version in ubuntu:
http://packages.ubuntu.com/trusty/media-ctl

So you can compile ubuntu dsc package, should work on debian.

-- 
Pali Rohár
pali.ro...@gmail.com
--
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: [RFC PATCH 00/24] Make Nokia N900 cameras working

[Pali Rohár]

front camera:

On Monday 25 April 2016 00:08:00 Ivaylo Dimitrov wrote:
> The needed pipeline could be made with:
> 
> media-ctl -r
> media-ctl -l '"vs6555 binner 2-0010":1 -> "video-bus-switch":2 [1]'
> media-ctl -l '"video-bus-switch":0 -> "OMAP3 ISP CCP2":0 [1]'
> media-ctl -l '"OMAP3 ISP CCP2":1 -> "OMAP3 ISP CCDC":0 [1]'
> media-ctl -l '"OMAP3 ISP CCDC":2 -> "OMAP3 ISP preview":0 [1]'
> media-ctl -l '"OMAP3 ISP preview":1 -> "OMAP3 ISP resizer":0 [1]'
> media-ctl -l '"OMAP3 ISP resizer":1 -> "OMAP3 ISP resizer output":0 [1]'
> media-ctl -V '"vs6555 pixel array 2-0010":0 [SGRBG10/648x488 (0,0)/648x488 
> (0,0)/648x488]'
> media-ctl -V '"vs6555 binner 2-0010":1 [SGRBG10/648x488 (0,0)/648x488 
> (0,0)/648x488]'
> media-ctl -V '"OMAP3 ISP CCP2":0 [SGRBG10 648x488]'
> media-ctl -V '"OMAP3 ISP CCP2":1 [SGRBG10 648x488]'
> media-ctl -V '"OMAP3 ISP CCDC":2 [SGRBG10 648x488]'
> media-ctl -V '"OMAP3 ISP preview":1 [UYVY 648x488]'
> media-ctl -V '"OMAP3 ISP resizer":1 [UYVY 656x488]'
> 
> and tested with:
> 
> mplayer -tv driver=v4l2:width=656:height=488:outfmt=uyvy:device=/dev/video6 
> -vo xv -vf screenshot tv://

back camera:

On Monday 25 April 2016 09:33:18 Ivaylo Dimitrov wrote:
> Try with:
> 
> media-ctl -r
> media-ctl -l '"et8ek8 3-003e":0 -> "video-bus-switch":1 [1]'
> media-ctl -l '"video-bus-switch":0 -> "OMAP3 ISP CCP2":0 [1]'
> media-ctl -l '"OMAP3 ISP CCP2":1 -> "OMAP3 ISP CCDC":0 [1]'
> media-ctl -l '"OMAP3 ISP CCDC":2 -> "OMAP3 ISP preview":0 [1]'
> media-ctl -l '"OMAP3 ISP preview":1 -> "OMAP3 ISP resizer":0 [1]'
> media-ctl -l '"OMAP3 ISP resizer":1 -> "OMAP3 ISP resizer output":0 [1]'
> 
> media-ctl -V '"et8ek8 3-003e":0 [SGRBG10 864x656]'
> media-ctl -V '"OMAP3 ISP CCP2":0 [SGRBG10 864x656]'
> media-ctl -V '"OMAP3 ISP CCP2":1 [SGRBG10 864x656]'
> media-ctl -V '"OMAP3 ISP CCDC":2 [SGRBG10 864x656]'
> media-ctl -V '"OMAP3 ISP preview":1 [UYVY 864x656]'
> media-ctl -V '"OMAP3 ISP resizer":1 [UYVY 800x600]'
> 
> 
> mplayer -tv driver=v4l2:width=800:height=600:outfmt=uyvy:device=/dev/video6 
> -vo xv -vf screenshot tv://

Hey!!! That is crazy! Who created such retard API?? In both cases you
are going to show video from /dev/video6 device. But in real I have two
independent camera devices: front and back.

Why on the earth I cannot have /dev/video0 for back camera and
/dev/video1 for front camera? And need to call such huge commands which
re-route pictures from correct camera to /dev/video6 device? I'm really
not interested in some hw details how are cameras connected, I just want
to show pictures in userspace...

And what are those others /dev/video[0-5] devices?

-- 
Pali Rohár
pali.ro...@gmail.com
--
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] Staging: media/bcm2048: Fix line over 80 characters warning as detected by checkpatch.pl

[Pali Rohár]

On Wednesday 12 August 2015 11:12:49 Shah, Yash (Y.) wrote:
 From: Yash Shah ysh...@visteon.com
 
 Fix line over 80 characters warning as detected by checkpatch.pl
 
 Signed-off-by: Yash Shah ysh...@visteon.com
 ---
  drivers/staging/media/bcm2048/radio-bcm2048.c | 3 ++-
  1 file changed, 2 insertions(+), 1 deletion(-)
 
 diff --git a/drivers/staging/media/bcm2048/radio-bcm2048.c 
 b/drivers/staging/media/bcm2048/radio-bcm2048.c
 index 8bc68e2..d36350e 100644
 --- a/drivers/staging/media/bcm2048/radio-bcm2048.c
 +++ b/drivers/staging/media/bcm2048/radio-bcm2048.c
 @@ -2243,7 +2243,8 @@ static ssize_t bcm2048_fops_read(struct file *file, 
 char __user *buf,
  
   tmpbuf[i] = bdev-rds_info.radio_text[bdev-rd_index+i+2];
   tmpbuf[i+1] = bdev-rds_info.radio_text[bdev-rd_index+i+1];
 - tmpbuf[i+2] = (bdev-rds_info.radio_text[bdev-rd_index + i]  
 0xf0)  4;
 + tmpbuf[i+2] = (bdev-rds_info.radio_text[bdev-rd_index + i]
 +   0xf0)  4;
   if ((bdev-rds_info.radio_text[bdev-rd_index+i] 
   BCM2048_RDS_CRC_MASK) == BCM2048_RDS_CRC_UNRECOVARABLE)
   tmpbuf[i+2] |= 0x80;

Hi! I think that code after this change is less readable as before.

-- 
Pali Rohár
pali.ro...@gmail.com
--
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 v2] radio-bcm2048: Fix region selection

[Pali Rohár]

From: Jan Roemisch m...@spaceboyz.net

This patch fixes region selection for lower bottom_frequency in BCM2048 FM
receiver. It also removes Japan wide band region since this is impossible
to do just like that.

Signed-off-by: Jan Roemisch m...@spaceboyz.net
Acked-by: Pali Rohár pali.ro...@gmail.com
---
 drivers/staging/media/bcm2048/radio-bcm2048.c |   20 
 1 file changed, 12 insertions(+), 8 deletions(-)

diff --git a/drivers/staging/media/bcm2048/radio-bcm2048.c 
b/drivers/staging/media/bcm2048/radio-bcm2048.c
index e9d0691..134e2af 100644
--- a/drivers/staging/media/bcm2048/radio-bcm2048.c
+++ b/drivers/staging/media/bcm2048/radio-bcm2048.c
@@ -342,14 +342,6 @@ static struct region_info region_configs[] = {
.deemphasis = 50,
.region = 3,
},
-   /* Japan wide band */
-   {
-   .channel_spacing= 10,
-   .bottom_frequency   = 76000,
-   .top_frequency  = 108000,
-   .deemphasis = 50,
-   .region = 4,
-   },
 };
 
 /*
@@ -741,6 +733,18 @@ static int bcm2048_set_region(struct bcm2048_device *bdev, 
u8 region)
 
mutex_lock(bdev-mutex);
bdev-region_info = region_configs[region];
+
+   if (region_configs[region].bottom_frequency  87500)
+   bdev-cache_fm_ctrl |= BCM2048_BAND_SELECT;
+   else
+   bdev-cache_fm_ctrl = ~BCM2048_BAND_SELECT;
+
+   err = bcm2048_send_command(bdev, BCM2048_I2C_FM_CTRL,
+   bdev-cache_fm_ctrl);
+   if (err) {
+   mutex_unlock(bdev-mutex);
+   goto done;
+   }
mutex_unlock(bdev-mutex);
 
if (bdev-frequency  region_configs[region].bottom_frequency ||
-- 
1.7.10.4

--
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] radio-bcm2048: Fix region selection

[Pali Rohár]

On Friday 05 June 2015 13:36:40 Hans Verkuil wrote:
 On 05/15/2015 11:32 PM, Pali Rohár wrote:
  From: maxx m...@spaceboyz.net
  
  This actually fixes region selection for BCM2048 FM receiver. To select
  the japanese FM-band an additional bit in FM_CTRL register needs to be
  set. This might not sound so important but it enables at least me to
  listen to some 'very interesting' radio transmission below normal
  FM-band.
  
  Patch writen by m...@spaceboyz.net
  
  Signed-off-by: Pali Rohár pali.ro...@gmail.com
  Cc: m...@spaceboyz.net
 
 Looks good to me. If someone can repost with correct names and SoBs, then I'll
 apply.
 

Jan, will you resend patch in correct format with correct names?

-- 
Pali Rohár
pali.ro...@gmail.com
--
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] radio-bcm2048: Enable access to automute and ctrl registers

[Pali Rohár]

From: maxx m...@spaceboyz.net

This enables access to automute function of the chip via sysfs and
gives direct access to FM_AUDIO_CTRL0/1 registers, also via sysfs. I
don't think this is so important but helps in developing radio scanner
apps.

Patch writen by m...@spaceboyz.net

Signed-off-by: Pali Rohár pali.ro...@gmail.com
Cc: m...@spaceboyz.net
---
 drivers/staging/media/bcm2048/radio-bcm2048.c |   96 +
 1 file changed, 96 insertions(+)

diff --git a/drivers/staging/media/bcm2048/radio-bcm2048.c 
b/drivers/staging/media/bcm2048/radio-bcm2048.c
index 1482d4b..8f9ba7b 100644
--- a/drivers/staging/media/bcm2048/radio-bcm2048.c
+++ b/drivers/staging/media/bcm2048/radio-bcm2048.c
@@ -826,6 +826,93 @@ static int bcm2048_get_mute(struct bcm2048_device *bdev)
return err;
 }
 
+static int bcm2048_set_automute(struct bcm2048_device *bdev, u8 automute)
+{
+   int err;
+
+   mutex_lock(bdev-mutex);
+
+   err = bcm2048_send_command(bdev, BCM2048_I2C_FM_AUDIO_PAUSE, automute);
+
+   mutex_unlock(bdev-mutex);
+   return err;
+}
+
+static int bcm2048_get_automute(struct bcm2048_device *bdev)
+{
+   int err;
+   u8 value;
+
+   mutex_lock(bdev-mutex);
+
+   err = bcm2048_recv_command(bdev, BCM2048_I2C_FM_AUDIO_PAUSE, value);
+
+   mutex_unlock(bdev-mutex);
+
+   if (!err)
+   err = value;
+
+   return err;
+}
+
+static int bcm2048_set_ctrl0(struct bcm2048_device *bdev, u8 value)
+{
+   int err;
+
+   mutex_lock(bdev-mutex);
+
+   err = bcm2048_send_command(bdev, BCM2048_I2C_FM_AUDIO_CTRL0, value);
+
+   mutex_unlock(bdev-mutex);
+   return err;
+}
+
+static int bcm2048_set_ctrl1(struct bcm2048_device *bdev, u8 value)
+{
+   int err;
+
+   mutex_lock(bdev-mutex);
+
+   err = bcm2048_send_command(bdev, BCM2048_I2C_FM_AUDIO_CTRL1, value);
+
+   mutex_unlock(bdev-mutex);
+   return err;
+}
+
+static int bcm2048_get_ctrl0(struct bcm2048_device *bdev)
+{
+   int err;
+   u8 value;
+
+   mutex_lock(bdev-mutex);
+
+   err = bcm2048_recv_command(bdev, BCM2048_I2C_FM_AUDIO_CTRL0, value);
+
+   mutex_unlock(bdev-mutex);
+
+   if (!err)
+   err = value;
+
+   return err;
+}
+
+static int bcm2048_get_ctrl1(struct bcm2048_device *bdev)
+{
+   int err;
+   u8 value;
+
+   mutex_lock(bdev-mutex);
+
+   err = bcm2048_recv_command(bdev, BCM2048_I2C_FM_AUDIO_CTRL1, value);
+
+   mutex_unlock(bdev-mutex);
+
+   if (!err)
+   err = value;
+
+   return err;
+}
+
 static int bcm2048_set_audio_route(struct bcm2048_device *bdev, u8 route)
 {
int err;
@@ -2058,6 +2145,9 @@ static ssize_t bcm2048_##prop##_read(struct device *dev,  
\
 
 DEFINE_SYSFS_PROPERTY(power_state, unsigned, int, %u, 0)
 DEFINE_SYSFS_PROPERTY(mute, unsigned, int, %u, 0)
+DEFINE_SYSFS_PROPERTY(automute, unsigned, int, %x, 0)
+DEFINE_SYSFS_PROPERTY(ctrl0, unsigned, int, %x, 0)
+DEFINE_SYSFS_PROPERTY(ctrl1, unsigned, int, %x, 0)
 DEFINE_SYSFS_PROPERTY(audio_route, unsigned, int, %u, 0)
 DEFINE_SYSFS_PROPERTY(dac_output, unsigned, int, %u, 0)
 
@@ -2095,6 +2185,12 @@ static struct device_attribute attrs[] = {
bcm2048_power_state_write),
__ATTR(mute, S_IRUGO | S_IWUSR, bcm2048_mute_read,
bcm2048_mute_write),
+   __ATTR(automute, S_IRUGO | S_IWUSR, bcm2048_automute_read,
+   bcm2048_automute_write),
+   __ATTR(ctrl0, S_IRUGO | S_IWUSR, bcm2048_ctrl0_read,
+   bcm2048_ctrl0_write),
+   __ATTR(ctrl1, S_IRUGO | S_IWUSR, bcm2048_ctrl1_read,
+   bcm2048_ctrl1_write),
__ATTR(audio_route, S_IRUGO | S_IWUSR, bcm2048_audio_route_read,
bcm2048_audio_route_write),
__ATTR(dac_output, S_IRUGO | S_IWUSR, bcm2048_dac_output_read,
-- 
1.7.9.5

--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] radio-bcm2048: Fix region selection

[Pali Rohár]

From: maxx m...@spaceboyz.net

This actually fixes region selection for BCM2048 FM receiver. To select
the japanese FM-band an additional bit in FM_CTRL register needs to be
set. This might not sound so important but it enables at least me to
listen to some 'very interesting' radio transmission below normal
FM-band.

Patch writen by m...@spaceboyz.net

Signed-off-by: Pali Rohár pali.ro...@gmail.com
Cc: m...@spaceboyz.net
---
 drivers/staging/media/bcm2048/radio-bcm2048.c |   13 +
 1 file changed, 13 insertions(+)

diff --git a/drivers/staging/media/bcm2048/radio-bcm2048.c 
b/drivers/staging/media/bcm2048/radio-bcm2048.c
index aeb6c3c..1482d4b 100644
--- a/drivers/staging/media/bcm2048/radio-bcm2048.c
+++ b/drivers/staging/media/bcm2048/radio-bcm2048.c
@@ -739,7 +739,20 @@ static int bcm2048_set_region(struct bcm2048_device *bdev, 
u8 region)
return -EINVAL;
 
mutex_lock(bdev-mutex);
+
bdev-region_info = region_configs[region];
+
+   bdev-cache_fm_ctrl = ~BCM2048_BAND_SELECT;
+   if (region  2) {
+   bdev-cache_fm_ctrl |= BCM2048_BAND_SELECT;
+   err = bcm2048_send_command(bdev, BCM2048_I2C_FM_CTRL,
+   bdev-cache_fm_ctrl);
+   if (err) {
+   mutex_unlock(bdev-mutex);
+   goto done;
+   }
+   }
+
mutex_unlock(bdev-mutex);
 
if (bdev-frequency  region_configs[region].bottom_frequency ||
-- 
1.7.9.5

--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] radio-bcm2048: remove unused var

[Pali Rohár]

On Tuesday 28 April 2015 09:03:41 Mauro Carvalho Chehab wrote:
 drivers/staging/media/bcm2048/radio-bcm2048.c: In function 
 'bcm2048_i2c_driver_probe':
 drivers/staging/media/bcm2048/radio-bcm2048.c:2596:11: warning: variable 
 'skip_release' set but not used [-Wunused-but-set-variable]
   int err, skip_release = 0;
^
 
 Signed-off-by: Mauro Carvalho Chehab mche...@osg.samsung.com
 
 diff --git a/drivers/staging/media/bcm2048/radio-bcm2048.c 
 b/drivers/staging/media/bcm2048/radio-bcm2048.c
 index e9d0691b21d3..5e11a78ceef3 100644
 --- a/drivers/staging/media/bcm2048/radio-bcm2048.c
 +++ b/drivers/staging/media/bcm2048/radio-bcm2048.c
 @@ -2593,7 +2593,7 @@ static int bcm2048_i2c_driver_probe(struct i2c_client 
 *client,
   const struct i2c_device_id *id)
  {
   struct bcm2048_device *bdev;
 - int err, skip_release = 0;
 + int err;
  
   bdev = kzalloc(sizeof(*bdev), GFP_KERNEL);
   if (!bdev) {
 @@ -2646,7 +2646,6 @@ free_sysfs:
   bcm2048_sysfs_unregister_properties(bdev, ARRAY_SIZE(attrs));
  free_registration:
   video_unregister_device(bdev-videodev);
 - skip_release = 1;
  free_irq:
   if (client-irq)
   free_irq(client-irq, bdev);

Looks good to me, so

Acked-by: Pali Rohár pali.ro...@gmail.com

-- 
Pali Rohár
pali.ro...@gmail.com
--
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: bcm2048: remove unused return of function

[Pali Rohár]

On Sunday 08 February 2015 23:29:11 Luis de Bethencourt wrote:
 Integer return of bcm2048_parse_rds_rt () is never used,
 changing the return type to void.
 
 Signed-off-by: Luis de Bethencourt luis...@samsung.com
 ---
  drivers/staging/media/bcm2048/radio-bcm2048.c | 4 +---
  1 file changed, 1 insertion(+), 3 deletions(-)
 

Looks good,

Acked-by: Pali Rohár pali.ro...@gmail.com

-- 
Pali Rohár
pali.ro...@gmail.com


signature.asc
Description: This is a digitally signed message part.

Re: [PATCH 2/5] radio-bcm2048: use unlocked_ioctl instead of ioctl

[Pali Rohár]

On Tuesday 03 February 2015 13:47:23 Hans Verkuil wrote:
 From: Hans Verkuil hans.verk...@cisco.com
 
 This driver does its own locking, so there is no need to use
 ioctl instead of unlocked_ioctl.
 
 Signed-off-by: Hans Verkuil hans.verk...@cisco.com

Looks good,
Acked-by: Pali Rohár pali.ro...@gmail.com

-- 
Pali Rohár
pali.ro...@gmail.com


signature.asc
Description: This is a digitally signed message part.

Re: [RFC] adp1653: Add device tree bindings for LED controller

[Pali Rohár]

On Sunday 16 November 2014 08:59:28 Pavel Machek wrote:
 For device tree people: Yes, I know I'll have to create file
 in documentation, but does the binding below look acceptable?
 
 I'll clean up driver code a bit more, remove the printks.
 Anything else obviously wrong?
 
 Signed-off-by: Pavel Machek pa...@ucw.cz
 
 Thanks,
   Pavel
 
 

Hello,

I think that this patch is probably not good and specially not 
for n900. adp1653 should be registered throw omap3 isp camera 
subsystem which does not have DT support yet.

See n900 legacy board camera code in file board-rx51-camera.c.

-- 
Pali Rohár
pali.ro...@gmail.com


signature.asc
Description: This is a digitally signed message part.

Re: [RFC] adp1653: Add device tree bindings for LED controller

[Pali Rohár]

On Monday 17 November 2014 11:05:19 Pavel Machek wrote:
 Hi!
 
 On Mon 2014-11-17 09:43:19, Pali Rohár wrote:
  On Sunday 16 November 2014 08:59:28 Pavel Machek wrote:
   For device tree people: Yes, I know I'll have to create
   file in documentation, but does the binding below look
   acceptable?
   
   I'll clean up driver code a bit more, remove the printks.
   Anything else obviously wrong?
  
  I think that this patch is probably not good and specially
  not for n900. adp1653 should be registered throw omap3 isp
  camera subsystem which does not have DT support yet.
 
 Can you explain?
 
 adp1653 is independend device on i2c bus, and we have kernel
 driver for it (unlike rest of n900 camera system). Just now
 it is unusable due to lack of DT binding. It has two
 functions, LED light and a camera flash; yes, the second one
 should be integrated to the rest of camera system, but that
 is not yet merged. That should not prevent us from merging DT
 support for the flash, so that this part can be
 tested/maintained.
 

Ok. When ISP camera subsystem has DT support somebody will modify 
n900 DT to add camera flash from adp1653 to ISP... I believe it 
will not be hard.

  See n900 legacy board camera code in file
  board-rx51-camera.c.
 
 I have seen that.
   Pavel

-- 
Pali Rohár
pali.ro...@gmail.com


signature.asc
Description: This is a digitally signed message part.

Re: [RFC] adp1653: Add device tree bindings for LED controller

[Pali Rohár]

On Monday 17 November 2014 15:55:46 Tony Lindgren wrote:
 * Pavel Machek pa...@ucw.cz [141117 02:17]:
  On Mon 2014-11-17 11:09:45, Pali Rohár wrote:
   On Monday 17 November 2014 11:05:19 Pavel Machek wrote:
Hi!

On Mon 2014-11-17 09:43:19, Pali Rohár wrote:
 On Sunday 16 November 2014 08:59:28 Pavel Machek wrote:
  For device tree people: Yes, I know I'll have to
  create file in documentation, but does the binding
  below look acceptable?
  
  I'll clean up driver code a bit more, remove the
  printks. Anything else obviously wrong?
 
 I think that this patch is probably not good and
 specially not for n900. adp1653 should be registered
 throw omap3 isp camera subsystem which does not have
 DT support yet.

Can you explain?

adp1653 is independend device on i2c bus, and we have
kernel driver for it (unlike rest of n900 camera
system). Just now it is unusable due to lack of DT
binding. It has two functions, LED light and a camera
flash; yes, the second one should be integrated to the
rest of camera system, but that is not yet merged. That
should not prevent us from merging DT support for the
flash, so that this part can be tested/maintained.
   
   Ok. When ISP camera subsystem has DT support somebody will
   modify n900 DT to add camera flash from adp1653 to ISP...
   I believe it will not be hard.
  
  Exactly. And yes, I'd like to get complete camera support
  for n900 merged. But first step is make sure existing
  support does not break.
 
 There's nothing stopping us from initializing the camera code
 from pdata-quirks.c for now to keep it working. Certainly the
 binding should be added to the driver, but that removes a
 dependency to the legacy booting mode if things are otherwise
 working.
 
 Regards,
 
 Tony

Tony, legacy board code for n900 is not in mainline tree. And 
that omap3 camera subsystem for n900 is broken since 3.5 
kernel... (both Front and Back camera on n900 show only green 
picture).

-- 
Pali Rohár
pali.ro...@gmail.com


signature.asc
Description: This is a digitally signed message part.

Re: [RFC] adp1653: Add device tree bindings for LED controller

[Pali Rohár]

On Monday 17 November 2014 16:04:07 Sakari Ailus wrote:
 Hi Pali,
 
 On Mon, Nov 17, 2014 at 04:01:31PM +0100, Pali Rohár wrote:
  On Monday 17 November 2014 15:55:46 Tony Lindgren wrote:
   * Pavel Machek pa...@ucw.cz [141117 02:17]:
On Mon 2014-11-17 11:09:45, Pali Rohár wrote:
 On Monday 17 November 2014 11:05:19 Pavel Machek wrote:
  Hi!
  
  On Mon 2014-11-17 09:43:19, Pali Rohár wrote:
   On Sunday 16 November 2014 08:59:28 Pavel Machek 
wrote:
For device tree people: Yes, I know I'll have to
create file in documentation, but does the
binding below look acceptable?

I'll clean up driver code a bit more, remove the
printks. Anything else obviously wrong?
   
   I think that this patch is probably not good and
   specially not for n900. adp1653 should be
   registered throw omap3 isp camera subsystem which
   does not have DT support yet.
  
  Can you explain?
  
  adp1653 is independend device on i2c bus, and we
  have kernel driver for it (unlike rest of n900
  camera system). Just now it is unusable due to lack
  of DT binding. It has two functions, LED light and
  a camera flash; yes, the second one should be
  integrated to the rest of camera system, but that
  is not yet merged. That should not prevent us from
  merging DT support for the flash, so that this part
  can be tested/maintained.
 
 Ok. When ISP camera subsystem has DT support somebody
 will modify n900 DT to add camera flash from adp1653
 to ISP... I believe it will not be hard.

Exactly. And yes, I'd like to get complete camera
support for n900 merged. But first step is make sure
existing support does not break.
   
   There's nothing stopping us from initializing the camera
   code from pdata-quirks.c for now to keep it working.
   Certainly the binding should be added to the driver, but
   that removes a dependency to the legacy booting mode if
   things are otherwise working.
   
   Regards,
   
   Tony
  
  Tony, legacy board code for n900 is not in mainline tree.
  And that omap3 camera subsystem for n900 is broken since
  3.5 kernel... (both Front and Back camera on n900 show only
  green picture).
 
 Can you capture raw bayer images correctly? I assume green
 means YUV buffers that are all zero.
 
 Do you know more specifically which patch breaks it?

CCing freemangordon (Ivaylo Dimitrov). He tried to debug it 
months ago but without success. Should know more info about this 
problem.

I think that commit which broke it was not bisected...

-- 
Pali Rohár
pali.ro...@gmail.com


signature.asc
Description: This is a digitally signed message part.

Re: [RFC] adp1653: Add device tree bindings for LED controller

[Pali Rohár]

On Monday 17 November 2014 16:06:17 Tony Lindgren wrote:
 * Pali Rohár pali.ro...@gmail.com [141117 07:03]:
  On Monday 17 November 2014 15:55:46 Tony Lindgren wrote:
   There's nothing stopping us from initializing the camera
   code from pdata-quirks.c for now to keep it working.
   Certainly the binding should be added to the driver, but
   that removes a dependency to the legacy booting mode if
   things are otherwise working.
  
  Tony, legacy board code for n900 is not in mainline tree.
  And that omap3 camera subsystem for n900 is broken since
  3.5 kernel... (both Front and Back camera on n900 show only
  green picture).
 
 I'm still seeing the legacy board code for n900 in mainline
 tree :) It's deprecated, but still there.
 

Yes, it is there because conversion from board code to DT is not 
complete yet... It is slow progress but you can watch it on page 
http://elinux.org/N900 (last two columns).

 Are you maybe talking about some other piece of platform_data
 that's no longer in the mainline kernel?
 

Yes, about platform_data which were never in mainline kernel. 
Just only in other trees. That code is: camera subsystem (with 
all other devices), cellular modem, bluetooth, radio.

 No idea what might be wrong with the camera though.
 
 Regards,
 
 Tony

-- 
Pali Rohár
pali.ro...@gmail.com


signature.asc
Description: This is a digitally signed message part.

Re: [PATCH v2] radio-bcm2048.c: fix wrong overflow check

[Pali Rohár]

On Monday 05 May 2014 15:34:29 Jiri Kosina wrote:
 On Tue, 22 Apr 2014, Dan Carpenter wrote:
  From: Pali Rohár pali.ro...@gmail.com
  
  This patch fixes an off by one check in
  bcm2048_set_region().
  
  Reported-by: Dan Carpenter dan.carpen...@oracle.com
  Signed-off-by: Pali Rohár pali.ro...@gmail.com
  Signed-off-by: Pavel Machek pa...@ucw.cz
  Signed-off-by: Dan Carpenter dan.carpen...@oracle.com
  ---
  v2: Send it to the correct list.  Re-work the changelog.
  
  This patch has been floating around for four months but
  Pavel and Pali are knuckle-heads and don't know how to use
  get_maintainer.pl so they never send it to linux-media.
  
  Also Pali doesn't give reporter credit and Pavel steals
  authorship credit.
  
  Also when you try explain to them about how to send patches
  correctly they complain that they have been trying but it
  is too much work so now I have to do it.  During the past
  four months thousands of other people have been able to
  send patches in the correct format to the correct list but
  it is too difficult for Pavel and Pali...  *sigh*.
 
 Seems like it's not in linux-next as of today, so I am taking
 it now. Thanks,

I still do not see this patch in torvalds branch... So what is 
needed to include this security buffer overflow patch into 
mainline  stable kernels?

-- 
Pali Rohár
pali.ro...@gmail.com


signature.asc
Description: This is a digitally signed message part.

Re: [PATCH] radio-bcm2048.c: fix wrong overflow check

[Pali Rohár]

On Tuesday 22 April 2014 11:38:36 Pavel Machek wrote:
 On Tue 2014-04-22 12:16:56, Dan Carpenter wrote:
  On Tue, Apr 22, 2014 at 10:55:53AM +0200, Pali Rohár wrote:
   On Tuesday 22 April 2014 10:39:17 Dan Carpenter wrote:
On Sun, Apr 20, 2014 at 04:56:22PM +0200, Pavel Machek 
wrote:
 Fix wrong overflow check in radio-bcm2048.
 
 Signed-off-by: Pali Rohár pali.ro...@gmail.com
 Signed-off-by: Pavel Machek pa...@ucw.cz

Signed off means like you're signing a legal document to
show that you didn't do anything illegal when you
handled the patch.  Was this patch authored by Pali? 
If so, then use the From: header.

Btw, I reported this bug on Dec 10 last year.  It's
better that we fix it now than not fix it at all but we
could have done better.  Was the kbuild-zero-day bug
report format confusing or how could I have helped out
there?

regards,
dan carpenter
   
   Hello, I sent this patch months ago, but not generated by
   commmand git format-patch.
  
  You should still have recieved authorship credit instead of
  Pavel.  It's a newbie mistake which I have made myself. 
  Pavel, use the From: header to give authorship credit.  It
  goes on the first line of the email.
  
  Did you send it to the correct list?  This patch should have
  gone to linux-media@vger.kernel.org.  I see now that they
  are not CC'd.  Please resend it to the correct list.
 
 How many more mails need to be generated for single line
 trivial patch? It is staging driver, so Greg should take it.
 Anyway, cc-ed the list now.
 
   And yes, this problem was reported by some public static
   code checker.
  
  I was the public static code checker and I sent the bug
  report from my @oracle.com email address.  Please, give me
  a Reported-by credit since you are resending this patch
  anyway.
 
 Feel free to resubmit the patch yourself.
 
   Pavel

I agree with Pavel, this patch which fixing buffer overflow bug 
should have been already included in kernel tree. And I think it 
really does not matter which from, to or cc lines are specified 
for singleline patch which was inspirated by static code checker. 
Rather to have fixed bug as talking who found it or who fixed it.

-- 
Pali Rohár
pali.ro...@gmail.com


signature.asc
Description: This is a digitally signed message part.

Re: [PATCH] media: Add BCM2048 radio driver

[Pali Rohár]

On Thursday 05 December 2013 08:52:30 Hans Verkuil wrote:
 On 12/02/2013 09:51 PM, Pali Rohár wrote:
  On Monday 04 November 2013 12:39:44 Hans Verkuil wrote:
  Hi Pali,
  
  On 10/26/2013 10:45 PM, Pali Rohár wrote:
  On Saturday 26 October 2013 22:22:09 Hans Verkuil wrote:
  Hans, so can it be added to drivers/staging/media tree?
  
  Yes, that is an option. It's up to you to decide what you
  want. Note that if no cleanup work is done on the staging
  driver for a long time, then it can be removed again.
  
  Regards,
  
  Hans
  
  Ok, so if you can add it to staging tree. When driver will
  be in mainline other developers can look at it too. Now
  when driver is hidden, nobody know where to find it... You
  can see how upstream development for Nokia N900 HW going
  on: http://elinux.org/N900
  
  Please check my tree:
  
  http://git.linuxtv.org/hverkuil/media_tree.git/shortlog/ref
  s/h eads/bcm
  
  If you're OK, then I'll queue it for 3.14 (it's too late
  for 3.13).
  
  Regards,
  
 Hans
  
  Hi, sorry for late reply. I looked into your tree and
  difference is that you only removed linux/slab.h include.
  So it it is not needed, then it is OK.
 
 I *added* slab.h :-)
 

Right, I looked at reverse diff :-)

 Anyway, I've posted the pull request. Please note, if you want
 to avoid having this driver be removed again in the future,
 then you (or someone else) should work on addressing the
 issues in the TODO file I added.
 
 Regards,
 
   Hans

Ok. CCing other people who works with n900 kernel.

-- 
Pali Rohár
pali.ro...@gmail.com


signature.asc
Description: This is a digitally signed message part.

Re: [PATCH] media: Add BCM2048 radio driver

[Pali Rohár]

On Monday 04 November 2013 12:39:44 Hans Verkuil wrote:
 Hi Pali,
 
 On 10/26/2013 10:45 PM, Pali Rohár wrote:
  On Saturday 26 October 2013 22:22:09 Hans Verkuil wrote:
  Hans, so can it be added to drivers/staging/media tree?
  
  Yes, that is an option. It's up to you to decide what you
  want. Note that if no cleanup work is done on the staging
  driver for a long time, then it can be removed again.
  
  Regards,
  
  Hans
  
  Ok, so if you can add it to staging tree. When driver will
  be in mainline other developers can look at it too. Now
  when driver is hidden, nobody know where to find it... You
  can see how upstream development for Nokia N900 HW going
  on: http://elinux.org/N900
 
 Please check my tree:
 
 http://git.linuxtv.org/hverkuil/media_tree.git/shortlog/refs/h
 eads/bcm
 
 If you're OK, then I'll queue it for 3.14 (it's too late for
 3.13).
 
 Regards,
 
   Hans

Hi, sorry for late reply. I looked into your tree and difference 
is that you only removed linux/slab.h include. So it it is not 
needed, then it is OK.

-- 
Pali Rohár
pali.ro...@gmail.com


signature.asc
Description: This is a digitally signed message part.

Re: [PATCH] media: Add BCM2048 radio driver

[Pali Rohár]

Hans, so can it be added to drivers/staging/media tree?

On Thursday 17 October 2013 21:31:04 Pali Rohár wrote:
 Hello,
 
 so what do you suggest? Add it to staging for now (or not)?
 
 On Tuesday 15 October 2013 17:08:39 Hans Verkuil wrote:
  Hi Pali,
  
  Thanks for the patch, but I am afraid it will need some work
  to make this acceptable for inclusion into the kernel.
  
  The main thing you need to do is to implement all the
  controls using the control framework (see
  Documentation/video4linux/v4l2-controls.txt). Most drivers
  are by now converted to the control framework, so you will
  find many examples of how to do this in drivers/media/radio.
  
  The sysfs stuff should be replaced by controls as well. A
  lot of the RDS support is now available as controls
  (although there may well be some missing features, but that
  is easy enough to add). Since the RDS data is actually
  read() from the device I am not sure whether the RDS
  properties/controls should be there at all.
  
  Finally this driver should probably be split up into two
  parts: one v4l2_subdev-based core driver and one platform
  driver. See e.g. radio-si4713/si4713-i2c.c as a good
  example. But I would wait with that until the rest of the
  driver is cleaned up. Then I have a better idea of whether
  this is necessary or not.
  
  It's also very useful to run v4l2-compliance (available in
  the v4l-utils.git repo on git.linuxtv.org). That does lots
  of sanity checks.
  
  Another option is to add the driver as-is to
  drivers/staging/media, and clean it up bit by bit.
  
  Regards,
  
  Hans
  
  On 10/15/2013 04:26 PM, Pali Rohár wrote:
   This adds support for the BCM2048 radio module found in
   Nokia N900
   
   Signed-off-by: Eero Nurkkala ext-eero.nurkk...@nokia.com
   Signed-off-by: Nils Faerber
   nils.faer...@kernelconcepts.de Signed-off-by: Joni
   Lapilainen joni.lapilai...@gmail.com Signed-off-by:
   Pali Rohár pali.ro...@gmail.com
   ---
   
drivers/media/radio/Kconfig |   10 +
drivers/media/radio/Makefile|1 +
drivers/media/radio/radio-bcm2048.c | 2744
+++
include/media/radio-bcm2048.h   |   30 +
4 files changed, 2785 insertions(+)
create mode 100644 drivers/media/radio/radio-bcm2048.c
create mode 100644 include/media/radio-bcm2048.h

-- 
Pali Rohár
pali.ro...@gmail.com


signature.asc
Description: This is a digitally signed message part.

Re: [PATCH] media: Add BCM2048 radio driver

[Pali Rohár]

On Saturday 26 October 2013 22:22:09 Hans Verkuil wrote:
  Hans, so can it be added to drivers/staging/media tree?
 
 Yes, that is an option. It's up to you to decide what you
 want. Note that if no cleanup work is done on the staging
 driver for a long time, then it can be removed again.
 
 Regards,
 
 Hans
 

Ok, so if you can add it to staging tree. When driver will be in 
mainline other developers can look at it too. Now when driver is 
hidden, nobody know where to find it... You can see how upstream 
development for Nokia N900 HW going on: http://elinux.org/N900

-- 
Pali Rohár
pali.ro...@gmail.com


signature.asc
Description: This is a digitally signed message part.

Re: [PATCH] media: Add BCM2048 radio driver

[Pali Rohár]

Hello,

so what do you suggest? Add it to staging for now (or not)?

On Tuesday 15 October 2013 17:08:39 Hans Verkuil wrote:
 Hi Pali,
 
 Thanks for the patch, but I am afraid it will need some work
 to make this acceptable for inclusion into the kernel.
 
 The main thing you need to do is to implement all the controls
 using the control framework (see
 Documentation/video4linux/v4l2-controls.txt). Most drivers
 are by now converted to the control framework, so you will
 find many examples of how to do this in drivers/media/radio.
 
 The sysfs stuff should be replaced by controls as well. A lot
 of the RDS support is now available as controls (although
 there may well be some missing features, but that is easy
 enough to add). Since the RDS data is actually read() from
 the device I am not sure whether the RDS properties/controls
 should be there at all.
 
 Finally this driver should probably be split up into two
 parts: one v4l2_subdev-based core driver and one platform
 driver. See e.g. radio-si4713/si4713-i2c.c as a good example.
 But I would wait with that until the rest of the driver is
 cleaned up. Then I have a better idea of whether this is
 necessary or not.
 
 It's also very useful to run v4l2-compliance (available in the
 v4l-utils.git repo on git.linuxtv.org). That does lots of
 sanity checks.
 
 Another option is to add the driver as-is to
 drivers/staging/media, and clean it up bit by bit.
 
 Regards,
 
   Hans
 
 On 10/15/2013 04:26 PM, Pali Rohár wrote:
  This adds support for the BCM2048 radio module found in
  Nokia N900
  
  Signed-off-by: Eero Nurkkala ext-eero.nurkk...@nokia.com
  Signed-off-by: Nils Faerber nils.faer...@kernelconcepts.de
  Signed-off-by: Joni Lapilainen joni.lapilai...@gmail.com
  Signed-off-by: Pali Rohár pali.ro...@gmail.com
  ---
  
   drivers/media/radio/Kconfig |   10 +
   drivers/media/radio/Makefile|1 +
   drivers/media/radio/radio-bcm2048.c | 2744
   +++
   include/media/radio-bcm2048.h   |   30 +
   4 files changed, 2785 insertions(+)
   create mode 100644 drivers/media/radio/radio-bcm2048.c
   create mode 100644 include/media/radio-bcm2048.h
  

-- 
Pali Rohár
pali.ro...@gmail.com


signature.asc
Description: This is a digitally signed message part.

[PATCH] rc: ir-rx51: Fix compilation

[Pali Rohár]

From: Joni Lapilainen joni.lapilai...@gmail.com

Signed-off-by: Joni Lapilainen joni.lapilai...@gmail.com
---
 drivers/media/rc/ir-rx51.c |5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/media/rc/ir-rx51.c b/drivers/media/rc/ir-rx51.c
index 8ead492..3971192 100644
--- a/drivers/media/rc/ir-rx51.c
+++ b/drivers/media/rc/ir-rx51.c
@@ -27,7 +27,8 @@
 #include linux/wait.h
 
 #include plat/dmtimer.h
-#include plat/clock.h
+
+#include ../../../arch/arm/mach-omap2/clock.h
 
 #include media/lirc.h
 #include media/lirc_dev.h
@@ -209,7 +210,7 @@ static int lirc_rx51_init_port(struct lirc_rx51 *lirc_rx51)
}
 
clk_fclk = omap_dm_timer_get_fclk(lirc_rx51-pwm_timer);
-   lirc_rx51-fclk_khz = clk_fclk-rate / 1000;
+   lirc_rx51-fclk_khz = clk_get_rate(clk_fclk) / 1000;
 
return 0;
 
-- 
1.7.10.4

--
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