[PATCH] MAINTAINERS: vivi -> vivid

2014-12-09 Thread Hans Verkuil
The vivi driver no longer exists and is replaced by the vivid driver.
Update MAINTAINERS accordingly.

Signed-off-by: Hans Verkuil 

---
diff --git a/MAINTAINERS b/MAINTAINERS
index 9c49eb6..6dc7f50 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -10032,13 +10032,13 @@ L:net...@vger.kernel.org
 S: Maintained
 F: drivers/net/ethernet/via/via-velocity.*
 
-VIVI VIRTUAL VIDEO DRIVER
+VIVID VIRTUAL VIDEO DRIVER
 M: Hans Verkuil 
 L: linux-media@vger.kernel.org
 T: git git://linuxtv.org/media_tree.git
 W: http://linuxtv.org
 S: Maintained
-F: drivers/media/platform/vivi*
+F: drivers/media/platform/vivid/*
 
 VLAN (802.1Q)
 M: Patrick McHardy 
--
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


cron job: media_tree daily build: OK

2014-12-09 Thread Hans Verkuil
This message is generated daily by a cron job that builds media_tree for
the kernels and architectures in the list below.

Results of the daily build of media_tree:

date:   Wed Dec 10 04:00:27 CET 2014
git branch: test
git hash:   71947828caef0c83d4245f7d1eaddc799b4ff1d1
gcc version:i686-linux-gcc (GCC) 4.9.1
sparse version: v0.5.0-35-gc1c3f96
smatch version: 0.4.1-3153-g7d56ab3
host hardware:  x86_64
host os:3.17-3.slh.2-amd64

linux-git-arm-at91: OK
linux-git-arm-davinci: OK
linux-git-arm-exynos: OK
linux-git-arm-mx: OK
linux-git-arm-omap: OK
linux-git-arm-omap1: OK
linux-git-arm-pxa: OK
linux-git-blackfin: OK
linux-git-i686: OK
linux-git-m32r: OK
linux-git-mips: OK
linux-git-powerpc64: OK
linux-git-sh: OK
linux-git-x86_64: OK
linux-2.6.32.27-i686: OK
linux-2.6.33.7-i686: OK
linux-2.6.34.7-i686: OK
linux-2.6.35.9-i686: OK
linux-2.6.36.4-i686: OK
linux-2.6.37.6-i686: OK
linux-2.6.38.8-i686: OK
linux-2.6.39.4-i686: OK
linux-3.0.60-i686: OK
linux-3.1.10-i686: OK
linux-3.2.37-i686: OK
linux-3.3.8-i686: OK
linux-3.4.27-i686: OK
linux-3.5.7-i686: OK
linux-3.6.11-i686: OK
linux-3.7.4-i686: OK
linux-3.8-i686: OK
linux-3.9.2-i686: OK
linux-3.10.1-i686: OK
linux-3.11.1-i686: OK
linux-3.12.23-i686: OK
linux-3.13.11-i686: OK
linux-3.14.9-i686: OK
linux-3.15.2-i686: OK
linux-3.16-i686: OK
linux-3.17-i686: OK
linux-3.18-i686: OK
linux-2.6.32.27-x86_64: OK
linux-2.6.33.7-x86_64: OK
linux-2.6.34.7-x86_64: OK
linux-2.6.35.9-x86_64: OK
linux-2.6.36.4-x86_64: OK
linux-2.6.37.6-x86_64: OK
linux-2.6.38.8-x86_64: OK
linux-2.6.39.4-x86_64: OK
linux-3.0.60-x86_64: OK
linux-3.1.10-x86_64: OK
linux-3.2.37-x86_64: OK
linux-3.3.8-x86_64: OK
linux-3.4.27-x86_64: OK
linux-3.5.7-x86_64: OK
linux-3.6.11-x86_64: OK
linux-3.7.4-x86_64: OK
linux-3.8-x86_64: OK
linux-3.9.2-x86_64: OK
linux-3.10.1-x86_64: OK
linux-3.11.1-x86_64: OK
linux-3.12.23-x86_64: OK
linux-3.13.11-x86_64: OK
linux-3.14.9-x86_64: OK
linux-3.15.2-x86_64: OK
linux-3.16-x86_64: OK
linux-3.17-x86_64: OK
linux-3.18-x86_64: OK
apps: OK
spec-git: OK
sparse: WARNINGS
smatch: ERRORS

Detailed results are available here:

http://www.xs4all.nl/~hverkuil/logs/Wednesday.log

Full logs are available here:

http://www.xs4all.nl/~hverkuil/logs/Wednesday.tar.bz2

The Media Infrastructure API from this daily build is here:

http://www.xs4all.nl/~hverkuil/spec/media.html
--
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: TT-connect CT2-4650 CI: DVB-C: no signal, no QAM

2014-12-09 Thread Pavol Domin
Hi Olli,

I guess it is OK that the media_build driver logs are from 3.17 (fedora) and
the technotrend ones from 3.16 (ubuntu). I would reply the scan
in the same kernel if not.

I've re-scaned again as you've suggested (the newest w_scan version). 

  1. newest media_build driver on 3.17.3-200.fc20.x86_64 kernel

"w_scan -v -v -v -fc -c SK -X" founds nothing. The full output is in
file w_scan_linux_media_log.txt.gz at 
https://drive.google.com/folderview?id=0B94Ll0t460PoTF9rWnEyTkdkRXc&usp=sharing

Curiously, czap now returns no 'not a QAM' error and looks perfectly OK (I call 
it the same way
as previosly), yet, I still cannot get any stream:
$ czap -r -c channels.xine.conf 'Eurosport HD'
using '/dev/dvb/adapter0/frontend0' and '/dev/dvb/adapter0/demux0'
reading channels from file 'channels.xine.conf'
141 Eurosport
HD:56200:INVERSION_AUTO:690:FEC_NONE:QAM_256:3000:3201:14001
141 Eurosport HD: f 56200, s 690, i 2, fec 0, qam 5, v 0xbb8, a 0xc81, 
s 0x36b1 
status 00 | signal  | snr  | ber  | unc  | 
status 1f | signal  | snr  | ber  | unc  | FE_HAS_LOCK
status 1f | signal  | snr  | ber  | unc  | FE_HAS_LOCK
status 1f | signal  | snr  | ber  | unc  | FE_HAS_LOCK
status 1f | signal  | snr  | ber  | unc  | FE_HAS_LOCK
status 1f | signal  | snr  | ber  | unc  | FE_HAS_LOCK
^C
$ czap -r -c channels.xine.conf 'STV1'
using '/dev/dvb/adapter0/frontend0' and '/dev/dvb/adapter0/demux0'
reading channels from file 'channels.xine.conf'
  5 STV1:37000:INVERSION_AUTO:690:FEC_NONE:QAM_256:33:310:1002
  5 STV1: f 37000, s 690, i 2, fec 0, qam 5, v 0x21, a 0x136, s 0x3ea 
status 00 | signal  | snr  | ber  | unc  | 
status 1f | signal  | snr  | ber  | unc  | FE_HAS_LOCK
status 1f | signal  | snr  | ber  | unc  | FE_HAS_LOCK
status 1f | signal  | snr  | ber  | unc  | FE_HAS_LOCK

$ time dvbdate
dvbdate: Unable to get time from multiplex.

real0m25.028s
user0m0.000s
sys 0m0.002s
$ time dvbtraffic
^C
real1m7.210s
user0m0.000s
sys 0m0.003s

Worth adding, green led turns on, while running czap.

  2. the manufacturer driver (3.16 ubuntu).

The same results, log is in the same share
https://drive.google.com/folderview?id=0B94Ll0t460PoTF9rWnEyTkdkRXc&usp=sharing
named w_scan_technotrend_log.txt.gz

Thanks for looking into this!

Regards

Pavol


On Tue, Dec 09, 2014 at 12:37:55PM +0700, Olli Salonen wrote:
> Hi Pavol,
> 
> Thanks. As said, I have not had any time to look into this, but will
> definitely do so. I own the same device, but do not have cable TV at
> home. Am using Conax CAM also successfully, so I believe that CI is
> not the issue.
> 
> Some things that came to my mind still:
> 
> Can you share the results of w_scan with very verbose output with both
> TT driver and the kernel driver? Also, make sure you use a recent
> version of w_scan - some distributions come with a rather old
> version...
> 
> w_scan -v -v -v  2>&1 | tee logfile.txt
> 
> Also, if you want to try a later firmware for Si2168, have a look at this 
> patch:
> http://git.linuxtv.org/cgit.cgi/media_tree.git/commit/?id=1b97dc98b58dad98f13fa0a4cdc819b60f3f3bff
> 
> It is in media_tree already.
> 
> Cheers,
> -olli
> 
> 
> On 8 December 2014 at 18:56, Pavol Domin  wrote:
> > Hi Olli,
> >
> > Thanks for feedback.
> >> Are you able to provide me a trace of the USB bus when using Windows? This
> >> is what I have been doing.
> >>
> >> 1) install USBlyzer
> >> 2) start it and select the option Capture hot plugged in the menus
> >> 3) start capture
> >> 4) plug in the device
> >> 5) start watching tv
> >> 6) stop capture after 1 sec to avoid the capture file growing too much
> > I've done that and shared at:
> > https://drive.google.com/open?id=0B94Ll0t460PoSTdKR0xiZlU2S0E&authuser=0
> >
> >> Would be also good to know if gnutv -cammenu works with the open source
> > Yes, it seems to work (except it coredumps after ctrl-c on fedora), e.g.
> > $ gnutv -channels channels.xine.conf -cammenu "Eurosport HD"
> > CAM Application type: 01
> > CAM Application manufacturer: 0b00
> > CAM Manufacturer code: 0001
> > CAM Menu string: Conax Conditional Access
> > CAM supports the following ca system ids:
> >   0x0b00
> >   --
> >   Conax Conditional Access
> >   Main menu
> >   0. Quit menu
> >   1. Subscription status
> >   2. Event status
> >   3. Tokens status
> >   4. Change CA PIN
> >   5. Maturity Rating
> >   6. Ordering online
> >   7. About Conax CA
> >   8. Messages
> >   9. Language
> >   10. Loader status
> >   11. CI Plus Info
> >   Press OK to select, or press RETURN
> >
> >> driver. Are all your channels encrypted? Is there any difference between
> >> them?
> > No, some are unencrypted. I cannot tell there are some other
> > differences, win

Re: [PATCH v6] media: platform: add VPFE capture driver support for AM437X

2014-12-09 Thread Hans Verkuil
On 12/09/2014 08:43 PM, Lad, Prabhakar wrote:
> From: Benoit Parrot 
> 
> This patch adds Video Processing Front End (VPFE) driver for
> AM437X family of devices
> Driver supports the following:
> - V4L2 API using MMAP buffer access based on videobuf2 api
> - Asynchronous sensor/decoder sub device registration
> - DT support
> 
> Signed-off-by: Benoit Parrot 
> Signed-off-by: Darren Etheridge 
> Signed-off-by: Lad, Prabhakar 
> ---
>  Changes for v6:
>  1: Fixed review comments pointed Hans, fixing race condition.
>  
>  v5: https://patchwork.kernel.org/patch/5459311/
>  v4: https://patchwork.kernel.org/patch/5449211/
>  v3: https://patchwork.kernel.org/patch/5434291/
>  v2: https://patchwork.kernel.org/patch/5425631/
>  v1: https://patchwork.kernel.org/patch/5362661/
>  
>  .../devicetree/bindings/media/ti-am437x-vpfe.txt   |   61 +
>  MAINTAINERS|9 +
>  drivers/media/platform/Kconfig |1 +
>  drivers/media/platform/Makefile|2 +
>  drivers/media/platform/am437x/Kconfig  |   11 +
>  drivers/media/platform/am437x/Makefile |3 +
>  drivers/media/platform/am437x/am437x-vpfe.c| 2777 
> 
>  drivers/media/platform/am437x/am437x-vpfe.h|  283 ++
>  drivers/media/platform/am437x/am437x-vpfe_regs.h   |  140 +
>  include/uapi/linux/Kbuild  |1 +
>  include/uapi/linux/am437x-vpfe.h   |  122 +
>  11 files changed, 3410 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/media/ti-am437x-vpfe.txt
>  create mode 100644 drivers/media/platform/am437x/Kconfig
>  create mode 100644 drivers/media/platform/am437x/Makefile
>  create mode 100644 drivers/media/platform/am437x/am437x-vpfe.c
>  create mode 100644 drivers/media/platform/am437x/am437x-vpfe.h
>  create mode 100644 drivers/media/platform/am437x/am437x-vpfe_regs.h
>  create mode 100644 include/uapi/linux/am437x-vpfe.h
> 



> +/*
> + * vpfe_release : This function is based on the vb2_fop_release
> + * helper function.
> + * It has been augmented to handle module power management,
> + * by disabling/enabling h/w module fcntl clock when necessary.
> + */
> +static int vpfe_release(struct file *file)
> +{
> + struct vpfe_device *vpfe = video_drvdata(file);
> + int ret;
> +
> + mutex_lock(&vpfe->lock);
> +
> + ret = _vb2_fop_release(file, NULL);
> + if (v4l2_fh_is_singular_file(file))
> + vpfe_ccdc_close(&vpfe->ccdc, vpfe->pdev);

This function seems cursed since your _vb2_fop_release call should
go *after* the v4l2_fh_is_singular_file() check.

I've fixed that in the patch in my pull request, so there is no need
for a v7. I know you've tested that since that was the order in your v5
version of this patch.

Regards,

Hans

> +
> + mutex_unlock(&vpfe->lock);
> +
> + return ret;
> +}
> +
> +/*
> + * vpfe_open : This function is based on the v4l2_fh_open helper function.
> + * It has been augmented to handle module power management,
> + * by disabling/enabling h/w module fcntl clock when necessary.
> + */
> +static int vpfe_open(struct file *file)
> +{
> + struct vpfe_device *vpfe = video_drvdata(file);
> + int ret;
> +
> + mutex_lock(&vpfe->lock);
> +
> + ret = v4l2_fh_open(file);
> + if (ret) {
> + vpfe_err(vpfe, "v4l2_fh_open failed\n");
> + goto unlock;
> + }
> +
> + if (!v4l2_fh_is_singular_file(file))
> + goto unlock;
> +
> + if (vpfe_initialize_device(vpfe)) {
> + v4l2_fh_release(file);
> + ret = -ENODEV;
> + }
> +
> +unlock:
> + mutex_unlock(&vpfe->lock);
> + 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


[GIT PULL FOR v3.20] Add am437x driver

2014-12-09 Thread Hans Verkuil
Adds the new am437x TI driver.

Regards,

Hans

The following changes since commit 71947828caef0c83d4245f7d1eaddc799b4ff1d1:

  [media] mn88473: One function call less in mn88473_init() after error 
(2014-12-04 16:00:47 -0200)

are available in the git repository at:

  git://linuxtv.org/hverkuil/media_tree.git am437x

for you to fetch changes up to 035e534ddf8e9f2bc14520c3c09e627e42b3186c:

  media: platform: add VPFE capture driver support for AM437X (2014-12-09 
21:43:04 +0100)


Benoit Parrot (1):
  media: platform: add VPFE capture driver support for AM437X

 Documentation/devicetree/bindings/media/ti-am437x-vpfe.txt |   61 ++
 MAINTAINERS|9 +
 drivers/media/platform/Kconfig |1 +
 drivers/media/platform/Makefile|2 +
 drivers/media/platform/am437x/Kconfig  |   11 +
 drivers/media/platform/am437x/Makefile |3 +
 drivers/media/platform/am437x/am437x-vpfe.c| 2778 

 drivers/media/platform/am437x/am437x-vpfe.h|  283 +++
 drivers/media/platform/am437x/am437x-vpfe_regs.h   |  140 
 include/uapi/linux/Kbuild  |1 +
 include/uapi/linux/am437x-vpfe.h   |  122 +++
 11 files changed, 3411 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/media/ti-am437x-vpfe.txt
 create mode 100644 drivers/media/platform/am437x/Kconfig
 create mode 100644 drivers/media/platform/am437x/Makefile
 create mode 100644 drivers/media/platform/am437x/am437x-vpfe.c
 create mode 100644 drivers/media/platform/am437x/am437x-vpfe.h
 create mode 100644 drivers/media/platform/am437x/am437x-vpfe_regs.h
 create mode 100644 include/uapi/linux/am437x-vpfe.h
--
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 v6] media: platform: add VPFE capture driver support for AM437X

2014-12-09 Thread Lad, Prabhakar
From: Benoit Parrot 

This patch adds Video Processing Front End (VPFE) driver for
AM437X family of devices
Driver supports the following:
- V4L2 API using MMAP buffer access based on videobuf2 api
- Asynchronous sensor/decoder sub device registration
- DT support

Signed-off-by: Benoit Parrot 
Signed-off-by: Darren Etheridge 
Signed-off-by: Lad, Prabhakar 
---
 Changes for v6:
 1: Fixed review comments pointed Hans, fixing race condition.
 
 v5: https://patchwork.kernel.org/patch/5459311/
 v4: https://patchwork.kernel.org/patch/5449211/
 v3: https://patchwork.kernel.org/patch/5434291/
 v2: https://patchwork.kernel.org/patch/5425631/
 v1: https://patchwork.kernel.org/patch/5362661/
 
 .../devicetree/bindings/media/ti-am437x-vpfe.txt   |   61 +
 MAINTAINERS|9 +
 drivers/media/platform/Kconfig |1 +
 drivers/media/platform/Makefile|2 +
 drivers/media/platform/am437x/Kconfig  |   11 +
 drivers/media/platform/am437x/Makefile |3 +
 drivers/media/platform/am437x/am437x-vpfe.c| 2777 
 drivers/media/platform/am437x/am437x-vpfe.h|  283 ++
 drivers/media/platform/am437x/am437x-vpfe_regs.h   |  140 +
 include/uapi/linux/Kbuild  |1 +
 include/uapi/linux/am437x-vpfe.h   |  122 +
 11 files changed, 3410 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/media/ti-am437x-vpfe.txt
 create mode 100644 drivers/media/platform/am437x/Kconfig
 create mode 100644 drivers/media/platform/am437x/Makefile
 create mode 100644 drivers/media/platform/am437x/am437x-vpfe.c
 create mode 100644 drivers/media/platform/am437x/am437x-vpfe.h
 create mode 100644 drivers/media/platform/am437x/am437x-vpfe_regs.h
 create mode 100644 include/uapi/linux/am437x-vpfe.h

diff --git a/Documentation/devicetree/bindings/media/ti-am437x-vpfe.txt 
b/Documentation/devicetree/bindings/media/ti-am437x-vpfe.txt
new file mode 100644
index 000..3932e76
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/ti-am437x-vpfe.txt
@@ -0,0 +1,61 @@
+Texas Instruments AM437x CAMERA (VPFE)
+--
+
+The Video Processing Front End (VPFE) is a key component for image capture
+applications. The capture module provides the system interface and the
+processing capability to connect RAW image-sensor modules and video decoders
+to the AM437x device.
+
+Required properties:
+- compatible: must be "ti,am437x-vpfe"
+- reg: physical base address and length of the registers set for the device;
+- interrupts: should contain IRQ line for the VPFE;
+- ti,am437x-vpfe-interface: can be one of the following,
+   0 - Raw Bayer Interface.
+   1 - 8 Bit BT656 Interface.
+   2 - 10 Bit BT656 Interface.
+   3 - YCbCr 8 Bit Interface.
+   4 - YCbCr 16 Bit Interface.
+
+VPFE supports a single port node with parallel bus. It should contain one
+'port' child node with child 'endpoint' node. Please refer to the bindings
+defined in Documentation/devicetree/bindings/media/video-interfaces.txt.
+
+Example:
+   vpfe: vpfe@f0034000 {
+   compatible = "ti,am437x-vpfe";
+   reg = <0x48328000 0x2000>;
+   interrupts = ;
+
+   pinctrl-names = "default", "sleep";
+   pinctrl-0 = <&vpfe_pins_default>;
+   pinctrl-1 = <&vpfe_pins_sleep>;
+
+   port {
+   #address-cells = <1>;
+   #size-cells = <0>;
+
+   vpfe0_ep: endpoint {
+   remote-endpoint = <&ov2659_1>;
+   ti,am437x-vpfe-interface = <0>;
+   bus-width = <8>;
+   hsync-active = <0>;
+   vsync-active = <0>;
+   };
+   };
+   };
+
+   i2c1: i2c@4802a000 {
+
+   ov2659@30 {
+   compatible = "ti,ov2659";
+   reg = <0x30>;
+
+   port {
+   ov2659_1: endpoint {
+   remote-endpoint = <&vpfe0_ep>;
+   bus-width = <8>;
+   mclk-frequency = <1200>;
+   };
+   };
+   };
diff --git a/MAINTAINERS b/MAINTAINERS
index 9c49eb6..5b1cb1a 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -8543,6 +8543,15 @@ S:   Maintained
 F: drivers/media/platform/davinci/
 F: include/media/davinci/
 
+TI AM437X VPFE DRIVER
+M: Lad, Prabhakar 
+L: linux-media@vger.kernel.org
+W: http://linuxtv.org/
+Q: http://patchwork.linuxtv.org/project/linux-media/list/
+T: git git://linuxtv.org/mhadli/v4l-dvb-davinci_devices.git
+S: Maintained
+F: drivers/media/platform/am437x/
+
 SIS 190 ETHERNET DRIVER
 M:

Re: [PATCH] usb: hcd: get/put device and hcd for hcd_buffers()

2014-12-09 Thread 'Greg Kroah-Hartman'
On Tue, Dec 09, 2014 at 05:01:35PM +0100, Sebastian Andrzej Siewior wrote:
> On 12/09/2014 04:24 PM, 'Greg Kroah-Hartman' wrote:
> > On Mon, Dec 08, 2014 at 09:44:05AM +, David Laight wrote:
> >> From: Greg Kroah-Hartman
> >>> On Fri, Dec 05, 2014 at 09:03:57PM +0100, Sebastian Andrzej Siewior wrote:
>  Consider the following scenario:
>  - plugin a webcam
>  - play the stream via gst-launch-0.10 v4l2src device=/dev/video0
>  - remove the USB-HCD during playback via "rmmod $HCD"
> 
>  and now wait for the crash
> >>>
> >>> Which you deserve, why did you ever remove a kernel module?  That's racy
> >>> and _never_ recommended, which is why it never happens automatically and
> >>> only root can do it.
> >>
> >> Really drivers and subsystems should have the required locking (etc) to
> >> ensure that kernel modules can either be unloaded, or that the unload
> >> request itself fails if the device is busy.
> >>
> >> It shouldn't be considered a 'shoot self in foot' operation.
> >> OTOH there are likely to be bugs.
> > 
> > This is not always the case, sorry, removing a kernel module is a known
> > racy condition, and sometimes adding all of the locking required to try
> > to make it "safe" just isn't worth it overall, as this is something that
> > _only_ a developer does.
> 
> I wasn't are of that. rmmod does not mention this. Kconfig does not
> mention this and suggest y as default (for MODULE_UNLOAD) . rmmod -f
> likely causes problems but this is not the case here. If you want to
> avoid rmmod why not mark a driver that it is not safe to remove it? And
> why not make it work?

Because sometimes fixes to make rmmod work "properly" entail slowing
down the whole "normal" path.  There is inherit problems with the core
of the module unload code for all modules that are known, and are not
going to be fixed because this isn't something that really matters.

> You can unbind the HCD driver from the PCI-device via sysfs and this is
> not something not only a developer does. This "unbind" calls the remove
> function of the driver and the only difference between unbind and rmmod
> is that the module remains inserted (but this is no news for you).

If unbind causes a problem, it's the same problem that could happen if
the hardware is hot-unplugged (like on a PCI card.)  Stuff like that
_does_ need to be fixed, and if your test shows this is a problem, I am
all for fixing that.

thanks,

greg k-h
--
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


[GIT PULL for v3.19-rc1] media updates

2014-12-09 Thread Mauro Carvalho Chehab
Hi Linus,

Please pull from:
  git://git.kernel.org/pub/scm/linux/kernel/git/mchehab/linux-media 
tags/media/v3.19-rc1


For:
  - Two new dvb frontend drivers: mn88472 and mn88473;
  - A new driver for some PCIe DVBSky cards;
  - A new remote controller driver: meson-ir;
  - One LIRC staging driver got rewritten and promoted to mainstream:
igorplugusb;
  - A new tuner driver (m88rs6000t);
  - The old omap2 media driver got removed from staging. This driver uses
an old DMA API and it is likely broken on recent kernels. Nobody cared
enough to fix it;
  - Media bus format moved to a separate header, as DRM will also use the
definitions there;
  - mem2mem_testdev were renamed to vim2m, in order to use the same naming
convention taken by the other virtual test driver (vivid);
  - Added a new driver for coda SoC (coda-jpeg);
  - The cx88 driver got converted to use videobuf2 core;
  - Make DMABUF export buffer to work with DMA Scatter/Gather and Vmalloc
cores;
  - Lots of other fixes, improvements and cleanups on the drivers.

Thanks!
Mauro

The following changes since commit 206c5f60a3d902bc4b56dab2de3e88de5eb06108:

  Linux 3.18-rc4 (2014-11-09 14:55:29 -0800)

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/mchehab/linux-media 
tags/media/v3.19-rc1

for you to fetch changes up to 71947828caef0c83d4245f7d1eaddc799b4ff1d1:

  [media] mn88473: One function call less in mn88473_init() after error 
(2014-12-04 16:00:47 -0200)


media updates for v3.19-rc1


Amber Thrall (1):
  [media] Staging: media: lirc: cleaned up packet dump in 2 files

Andreas Ruprecht (1):
  [media] media: pci: smipcie: Fix dependency for DVB_SMIPCIE

Andrey Utkin (4):
  [media] solo6x10: clean up properly in stop_streaming
  [media] solo6x10: free DMA allocation when releasing encoder
  [media] solo6x10: bind start & stop of encoded frames processing thread 
to device (de)init
  [media] solo6x10: don't turn off/on encoder interrupt in processing loop

Antti Palosaari (34):
  [media] si2168: do not print device is warm every-time when opened
  [media] af9033: fix AF9033 DVBv3 signal strength measurement
  [media] af9033: improve read_signal_strength error handling slightly
  [media] af9033: return 0.1 dB DVBv3 SNR for AF9030 family
  [media] af9033: continue polling unless critical IO error
  [media] mn88472: Panasonic MN88472 demod driver (DVB-C only)
  [media] mn88472: correct attach symbol name
  [media] mn88472: add small delay to wait DVB-C lock
  [media] mn88472: rename mn88472_c.c => mn88472.c
  [media] mn88472: rename state to dev
  [media] mn88472: convert driver to I2C client
  [media] mn88472: Convert driver to I2C RegMap API
  [media] mn88472: implement DVB-T and DVB-T2
  [media] mn88472: move to staging
  [media] mn88472: add staging TODO
  [media] MAINTAINERS: add mn88472 (Panasonic MN88472)
  [media] mn88473: Panasonic MN88473 DVB-T/T2/C demod driver
  [media] mn88473: add support for DVB-T2
  [media] mn88473: implement DVB-T mode
  [media] mn88473: improve IF frequency and BW handling
  [media] mn88473: convert driver to I2C binding
  [media] mn88473: convert to RegMap API
  [media] mn88473: move to staging
  [media] mn88473: add staging TODO
  [media] MAINTAINERS: add mn88473 (Panasonic MN88473)
  [media] r820t: add DVB-C config
  [media] rtl28xxu: enable demod ADC only when needed
  [media] rtl2832: implement option to bypass slave demod TS
  [media] rtl28xxu: add support for Panasonic MN88472 slave demod
  [media] rtl28xxu: add support for Panasonic MN88473 slave demod
  [media] rtl28xxu: rename tuner I2C client pointer
  [media] rtl28xxu: remove unused SDR attach logic
  [media] rtl28xxu: add SDR module for devices having R828D tuner
  [media] rtl2832_sdr: control ADC

Arun Mankuzhi (2):
  [media] s5p-mfc: modify mfc wakeup sequence for V8
  [media] s5p-mfc: De-init MFC when watchdog kicks in

Austin Lund (1):
  [media] media/rc: Send sync space information on the lirc device

Behan Webster (1):
  [media] ti-fpe: LLVMLinux: Remove nested function from ti-vpe

Beniamino Galvani (3):
  [media] media: rc: meson: document device tree bindings
  [media] media: rc: add driver for Amlogic Meson IR remote receiver
  [media] ARM: dts: meson: add IR receiver node

Bimow Chen (2):
  [media] af9033: fix DVBv3 signal strength value not correct issue
  [media] af9033: fix DVBv3 snr value not correct issue

Boris BREZILLON (10):
  [media] Move mediabus format definition to a more standard place
  [media] v4l: Update subdev-formats doc with new MEDIA_BUS_FMT values
  [media] Make use of the new media_bus_format definitions
  [media] i2c: Ma

Re: [PATCH] usb: hcd: get/put device and hcd for hcd_buffers()

2014-12-09 Thread Sebastian Andrzej Siewior
On 12/09/2014 04:24 PM, 'Greg Kroah-Hartman' wrote:
> On Mon, Dec 08, 2014 at 09:44:05AM +, David Laight wrote:
>> From: Greg Kroah-Hartman
>>> On Fri, Dec 05, 2014 at 09:03:57PM +0100, Sebastian Andrzej Siewior wrote:
 Consider the following scenario:
 - plugin a webcam
 - play the stream via gst-launch-0.10 v4l2src device=/dev/video0
 - remove the USB-HCD during playback via "rmmod $HCD"

 and now wait for the crash
>>>
>>> Which you deserve, why did you ever remove a kernel module?  That's racy
>>> and _never_ recommended, which is why it never happens automatically and
>>> only root can do it.
>>
>> Really drivers and subsystems should have the required locking (etc) to
>> ensure that kernel modules can either be unloaded, or that the unload
>> request itself fails if the device is busy.
>>
>> It shouldn't be considered a 'shoot self in foot' operation.
>> OTOH there are likely to be bugs.
> 
> This is not always the case, sorry, removing a kernel module is a known
> racy condition, and sometimes adding all of the locking required to try
> to make it "safe" just isn't worth it overall, as this is something that
> _only_ a developer does.

I wasn't are of that. rmmod does not mention this. Kconfig does not
mention this and suggest y as default (for MODULE_UNLOAD) . rmmod -f
likely causes problems but this is not the case here. If you want to
avoid rmmod why not mark a driver that it is not safe to remove it? And
why not make it work?

You can unbind the HCD driver from the PCI-device via sysfs and this is
not something not only a developer does. This "unbind" calls the remove
function of the driver and the only difference between unbind and rmmod
is that the module remains inserted (but this is no news for you).

Now, this unbind happens if you choose to pass a PCI-device to a qemu
guest. This is a fairly common use-case for a non-developer since it is
quite easy to setup in virt-manager for instance. All you need is a
hardware with IOMMU support. I used this to get the usb.org testsuite
running in my Windows guest which needs access to EHCI registers). I
could also mention hacking on XHCI and not crashing the physical
machine if something goes south but then I would rise the developer
card again.

I even rmmod & modprobe my mmc controller on my notebook because for
some reason it does not work otherwise after a suspend + resume cycle
(and my motivation to look after this is quite low since I barely use
my notebook at all).

I am really surprised that you as a core developer and maintainer of
the drivers infrastructure say that one should not remove a driver.

> greg k-h

Sebastian
--
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/RFC v8 02/14] Documentation: leds: Add description of LED Flash class extension

2014-12-09 Thread Pavel Machek
On Tue 2014-12-09 09:54:06, Jacek Anaszewski wrote:
> Hi Pavel,
> 
> On 12/08/2014 09:18 PM, Pavel Machek wrote:
> >On Mon 2014-12-08 17:55:20, Jacek Anaszewski wrote:
> >>On 12/06/2014 01:43 PM, Pavel Machek wrote:
> >>>
> >The format of a sysfs attribute should be concise.
> >The error codes are generic and map directly to the V4L2 Flash
> >error codes.
> >
> 
> Actually I'd like to see those flash fault code defined in LED
> subsystem. And V4L2 will just include LED flash header file to use it.
> Because flash fault code is not for V4L2 specific but it's a feature
> of LED flash devices.
> 
> For clearing error code of flash devices, I think it depends on the
> hardware. If most of our LED flash is using reading to clear error
> code, we probably can make it simple as this now. But what if some
> other LED flash devices are using writing to clear error code? we
> should provide a API to that?
> >>>
> >>>Actually, we should provide API that makes sense, and that is easy to
> >>>use by userspace.
> >>>
> >>>I believe "read" is called read because it does not change anything,
> >>>and it should stay that way in /sysfs. You may want to talk to sysfs
> >>>maintainers if you plan on doing another semantics.
> >>
> >>How would you proceed in case of devices which clear their fault
> >>register upon I2C readout (e.g. AS3645)? In this case read does have
> >>a side effect. For such devices attribute semantics would have to be
> >>different than for the devices which don't clear faults on readout.
> >
> >No, semantics should be same for all devices.
> >
> >If device clears fault register during I2C readout, kernel will simply
> >gather faults in an variable, and clear them upon write to sysfs file.
> 
> This approach would require implementing additional mechanisms on
> both sides: LED Flash class core and a LED Flash class driver.
> In the former the sysfs attribute write permissions would have
> to be decided in the runtime and in the latter caching mechanism

Write attributes at runtime? Why? We can emulate sane and consistent
behaviour for all the controllers: read gives you list of faults,
write clears it. We can do it for all the controllers.

Only cost is few lines of code in the drivers where hardware clears
faults at read.

> would have to be implemented per driver. We would have to also
> consider how to approach the issue in case of sub-leds.

Actually.. sub-leds. That is one physical LED being connected to two
current sources at the same time, right? 

> The only reason for this overhead is trying to avoid side effects
> on reading sysfs attribute. After weighing the pros and cons,
> I am not sure if it is worthwhile.

I am pretty sure.

Pavel

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
--
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] usb: hcd: get/put device and hcd for hcd_buffers()

2014-12-09 Thread 'Greg Kroah-Hartman'
On Mon, Dec 08, 2014 at 09:44:05AM +, David Laight wrote:
> From: Greg Kroah-Hartman
> > On Fri, Dec 05, 2014 at 09:03:57PM +0100, Sebastian Andrzej Siewior wrote:
> > > Consider the following scenario:
> > > - plugin a webcam
> > > - play the stream via gst-launch-0.10 v4l2src device=/dev/video0
> > > - remove the USB-HCD during playback via "rmmod $HCD"
> > >
> > > and now wait for the crash
> > 
> > Which you deserve, why did you ever remove a kernel module?  That's racy
> > and _never_ recommended, which is why it never happens automatically and
> > only root can do it.
> 
> Really drivers and subsystems should have the required locking (etc) to
> ensure that kernel modules can either be unloaded, or that the unload
> request itself fails if the device is busy.
> 
> It shouldn't be considered a 'shoot self in foot' operation.
> OTOH there are likely to be bugs.

This is not always the case, sorry, removing a kernel module is a known
racy condition, and sometimes adding all of the locking required to try
to make it "safe" just isn't worth it overall, as this is something that
_only_ a developer does.

greg k-h
--
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/RFC v9 04/19] mfd: max77693: adjust max77693_led_platform_data

2014-12-09 Thread Sylwester Nawrocki
On 09/12/14 15:41, Lee Jones wrote:
 struct max77693_led_platform_data {
 > >>+  const char *label[2];
 > >>   u32 fleds[2];
 > >>   u32 iout_torch[2];for_each_available_child_of_node
 > >>   u32 iout_flash[2];
 > >>   u32 trigger[2];
 > >>   u32 trigger_type[2];
 > >>+  u32 flash_timeout[2];
 > >>   u32 num_leds;
 > >>   u32 boost_mode;
 > >>-  u32 flash_timeout;
 > >>   u32 boost_vout;
 > >>   u32 low_vsys;
 > >>+  struct device_node *sub_nodes[2];
>>> > >
>>> > >I haven't seen anyone do this before.  Why can't you use the 
>>> > >provided
>>> > >OF functions to traverse through your tree?
>> > 
>> > I use for_each_available_child_of_node when parsing DT node, but I
>> > need to cache the pointer to sub-node to be able to use it later
>> > when it needs to be passed to V4L2 sub-device which is then
>> > asynchronously matched by the phandle to sub-node.
>> > 
>> > If it is not well seen to cache it in the platform data then
>> > I will find different way to accomplish this.
> > >>>
> > >>>I haven't seen the end-driver for this, but why can't you use that
> > >>>device's of_node pointer?
 > >>
 > >>Maybe it is indeed a good idea. I could pass the of_node pointer
 > >>and the sub-led identifier to the V4L2 sub-device and there look
 > >>for the sub-node containing relevant identifier. The downside
 > >>would be only that for_each_available_child_of_node would
 > >>have to be called twice - in the led driver and in the V4L2 sub-device.
 > >>I think that we can live with it.
>>> > >
>>> > >Are the LED and V4L2 drivers children of this MFD?  If so, you can use
>>> > >the of_compatible attribute in struct mfd_cell to populate the each
>>> > >child's of_node dynamically i.e. the MFD core will do that for you.
>>> > >
>> > 
>> > V4L2 driver wraps LED driver. This way the LED device can be
>> > controlled with use of two interfaces - LED subsystem sysfs
>> > and V4L2 Flash. This is the aim of the whole patch set.
>> > 
>> > I've thought it over again and it seems that I will need to cache
>> > somewhere these sub_nodes pointers. They have to be easily accessible
>> > for the V4L2 sub-device as it can be asynchronously registered
>> > or unregistered within V4L2 media device. Sub-devices are matched
>> > basing on the sub-node phandle.
>
> Not quite getting this.  Can you explain this in another way please?

Only the LED controller driver is a child the MFD. The LED controller
can contain multiple outputs with a physical LED attached to it. AFAICS
this binding is modelling each such an output as a the LED's controller
node child node.

I'm not sure though why storing the device node pointers is required,
rather than traversing OF tree when needed.
I guess we only need the list of the node pointer to populate struct
v4l2_async_subdev array for v4l2_async_notifier_register() call ?


-- 
Regards,
Sylwester
--
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/RFC v9 04/19] mfd: max77693: adjust max77693_led_platform_data

2014-12-09 Thread Lee Jones
On Tue, 09 Dec 2014, Jacek Anaszewski wrote:

> On 12/09/2014 02:50 PM, Lee Jones wrote:
> >On Tue, 09 Dec 2014, Jacek Anaszewski wrote:
> >
> >>On 12/09/2014 11:04 AM, Lee Jones wrote:
> >>>On Tue, 09 Dec 2014, Jacek Anaszewski wrote:
> >>>
> On 12/09/2014 09:50 AM, Lee Jones wrote:
> >On Wed, 03 Dec 2014, Jacek Anaszewski wrote:
> >
> >>Add "label" array for Device Tree strings with the name of a LED device
> >>and make flash_timeout a two element array, for caching the sub-led
> >>related flash timeout. Added is also an array for caching pointers to 
> >>the
> >>sub-nodes representing sub-leds.
> >>
> >>Signed-off-by: Jacek Anaszewski 
> >>Acked-by: Kyungmin Park 
> >>Cc: Chanwoo Choi 
> >>Cc: Lee Jones 
> >>---
> >>  include/linux/mfd/max77693.h |4 +++-
> >>  1 file changed, 3 insertions(+), 1 deletion(-)
> >>
> >>diff --git a/include/linux/mfd/max77693.h b/include/linux/mfd/max77693.h
> >>index f0b6585..c80ee99 100644
> >>--- a/include/linux/mfd/max77693.h
> >>+++ b/include/linux/mfd/max77693.h
> >>@@ -88,16 +88,18 @@ enum max77693_led_boost_mode {
> >>  };
> >>
> >>  struct max77693_led_platform_data {
> >>+   const char *label[2];
> >>u32 fleds[2];
> >>u32 iout_torch[2];for_each_available_child_of_node
> >>u32 iout_flash[2];
> >>u32 trigger[2];
> >>u32 trigger_type[2];
> >>+   u32 flash_timeout[2];
> >>u32 num_leds;
> >>u32 boost_mode;
> >>-   u32 flash_timeout;
> >>u32 boost_vout;
> >>u32 low_vsys;
> >>+   struct device_node *sub_nodes[2];
> >
> >I haven't seen anyone do this before.  Why can't you use the provided
> >OF functions to traverse through your tree?
> 
> I use for_each_available_child_of_node when parsing DT node, but I
> need to cache the pointer to sub-node to be able to use it later
> when it needs to be passed to V4L2 sub-device which is then
> asynchronously matched by the phandle to sub-node.
> 
> If it is not well seen to cache it in the platform data then
> I will find different way to accomplish this.
> >>>
> >>>I haven't seen the end-driver for this, but why can't you use that
> >>>device's of_node pointer?
> >>
> >>Maybe it is indeed a good idea. I could pass the of_node pointer
> >>and the sub-led identifier to the V4L2 sub-device and there look
> >>for the sub-node containing relevant identifier. The downside
> >>would be only that for_each_available_child_of_node would
> >>have to be called twice - in the led driver and in the V4L2 sub-device.
> >>I think that we can live with it.
> >
> >Are the LED and V4L2 drivers children of this MFD?  If so, you can use
> >the of_compatible attribute in struct mfd_cell to populate the each
> >child's of_node dynamically i.e. the MFD core will do that for you.
> >
> 
> V4L2 driver wraps LED driver. This way the LED device can be
> controlled with use of two interfaces - LED subsystem sysfs
> and V4L2 Flash. This is the aim of the whole patch set.
> 
> I've thought it over again and it seems that I will need to cache
> somewhere these sub_nodes pointers. They have to be easily accessible
> for the V4L2 sub-device as it can be asynchronously registered
> or unregistered within V4L2 media device. Sub-devices are matched
> basing on the sub-node phandle.

Not quite getting this.  Can you explain this in another way please?

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
--
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: FOSDEM15: Graphics DevRoom: call for speakers.

2014-12-09 Thread Luc Verhaegen
On Thu, Oct 02, 2014 at 07:44:57PM +0200, Luc Verhaegen wrote:
> Hi,
> 
> At FOSDEM on the 31st of january and the 1st of February 2015, there 
> will be another graphics DevRoom. URL: https://fosdem.org/2015/

> Slots will be handed out on a first come, first serve basis. The best 
> slots will go to those who apply the earliest. The amount of slots is 
> currently not known yet, but i expect there to be around 16 available (8 
> on each day), so act quickly.

> As for deadlines, i hope to have a pretty much complete schedule between 
> christmas and the new year. The rockhard printed schedule deadline is 
> probably January 9th, after that you will not be featured in the booklet 
> and you will have a lot less visitors. I will hopefully be able to lock 
> down entries and descriptions after that date.

It's been more than 2 months since the original email, it's less than 
two months away from the event, and one month away from what usually is 
the deadline for the booklet. File your talk now, while there are still 
some useful slots available.

Also, for those who have filed already but who have left their abstracts 
open, please get those filed in ASAP. Your talk will be only be ordered 
in when at least the basics are provided.

Thanks,

Luc Verhaegen.
--
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/RFC v9 06/19] DT: Add documentation for the mfd Maxim max77693

2014-12-09 Thread Jacek Anaszewski

Hi Sakari,

On 12/09/2014 03:09 PM, Sakari Ailus wrote:

Hi Jacek,

On Thu, Dec 04, 2014 at 12:40:48PM +0100, Jacek Anaszewski wrote:

+   the flash/torch.
+- maxim,trigger : Array of flags indicating which trigger can activate given 
led
+   in order: fled1, fled2.
+   Possible flag values (can be combined):
+   MAX77693_LED_TRIG_FLASHEN - FLASHEN pin of the chip,
+   MAX77693_LED_TRIG_TORCHEN - TORCHEN pin of the chip,
+   MAX77693_LED_TRIG_SOFTWARE - software via I2C command.


Is there a need to prevent strobing using a certain method? Just wondering.


In some cases it could be convenient to prevent some options through
device tree.


Do you have that need now?

If not, I'd propose to postpone this and add it only if there ever is one.



No, I don't. So let's postpone it.

Best Regards,
Jacek Anaszewski
--
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/RFC v9 01/19] leds: Add LED Flash class extension to the LED subsystem

2014-12-09 Thread Sakari Ailus
Hi Jacek,

On Tue, Dec 09, 2014 at 01:56:47PM +0100, Jacek Anaszewski wrote:
> Hi Sakari,
> 
> On 12/09/2014 01:36 PM, Sakari Ailus wrote:
> >Hi Jacek,
> >
> >On Thu, Dec 04, 2014 at 10:29:10AM +0100, Jacek Anaszewski wrote:
> >...
> +static struct attribute *led_flash_strobe_attrs[] = {
> + &dev_attr_flash_strobe.attr,
> + NULL,
> +};
> +
> +static struct attribute *led_flash_timeout_attrs[] = {
> + &dev_attr_flash_timeout.attr,
> + &dev_attr_max_flash_timeout.attr,
> + NULL,
> +};
> +
> +static struct attribute *led_flash_brightness_attrs[] = {
> + &dev_attr_flash_brightness.attr,
> + &dev_attr_max_flash_brightness.attr,
> + NULL,
> +};
> +
> +static struct attribute *led_flash_fault_attrs[] = {
> + &dev_attr_flash_fault.attr,
> + NULL,
> +};
> +
> +static struct attribute *led_flash_sync_strobe_attrs[] = {
> + &dev_attr_flash_sync_strobe.attr,
> + NULL,
> +};
> +
> +static const struct attribute_group led_flash_strobe_group = {
> + .attrs = led_flash_strobe_attrs,
> +};
> +
> +static const struct attribute_group led_flash_timeout_group = {
> + .attrs = led_flash_timeout_attrs,
> +};
> +
> +static const struct attribute_group led_flash_brightness_group = {
> + .attrs = led_flash_brightness_attrs,
> +};
> +
> +static const struct attribute_group led_flash_fault_group = {
> + .attrs = led_flash_fault_attrs,
> +};
> +
> +static const struct attribute_group led_flash_sync_strobe_group = {
> + .attrs = led_flash_sync_strobe_attrs,
> +};
> +
> +static const struct attribute_group *flash_groups[] = {
> + &led_flash_strobe_group,
> + NULL,
> + NULL,
> + NULL,
> + NULL,
> + NULL,
> + NULL
> +};
> +
> +static void led_flash_resume(struct led_classdev *led_cdev)
> +{
> + struct led_classdev_flash *flash = lcdev_to_flash(led_cdev);
> +
> + call_flash_op(flash, flash_brightness_set, flash->brightness.val);
> + call_flash_op(flash, timeout_set, flash->timeout.val);
> +}
> +
> +static void led_flash_init_sysfs_groups(struct led_classdev_flash *flash)
> +{
> + struct led_classdev *led_cdev = &flash->led_cdev;
> + const struct led_flash_ops *ops = flash->ops;
> + int num_sysfs_groups = 1;
> +
> + if (ops->flash_brightness_set)
> + flash_groups[num_sysfs_groups++] = &led_flash_brightness_group;
> +
> + if (ops->timeout_set)
> + flash_groups[num_sysfs_groups++] = &led_flash_timeout_group;
> +
> + if (ops->fault_get)
> + flash_groups[num_sysfs_groups++] = &led_flash_fault_group;
> +
> + if (led_cdev->flags & LED_DEV_CAP_COMPOUND)
> + flash_groups[num_sysfs_groups++] = &led_flash_sync_strobe_group;
> +
> + led_cdev->groups = flash_groups;
> >>>
> >>>Shouldn't you have groups local to the device instead? If you register
> >>>another flash device bad things will happen if the ops the device supports
> >>>are different.
> >>
> >>The groups are local to the device. A LED class device is registered
> >>with device_create_with_groups called from led_classdev_register
> >>function. It is passed led_cdev->groups in the fifth argument.
> >
> >The groups pointer will be stored in struct device. If you have another
> >driver using different groups, it will affect the groups for all flash
> >devices that use the same groups pointer. I'm not sure what exactly would
> >follow from that but I'd rather not change them once the device is created.
> 
> I had to take another look at this to understand the problem.
> I think that the best option will be making flash_groups array
> a member of struct led_classdev_flash.

Sounds good to me.

> +}
> +
> +int led_classdev_flash_register(struct device *parent,
> + struct led_classdev_flash *flash)
> +{
> + struct led_classdev *led_cdev;
> + const struct led_flash_ops *ops;
> + int ret;
> +
> + if (!flash)
> >>>
> >>>Do you have a use case for this?
> >>
> >>This is just a guard against NULL pointer dereference. Maybe it is
> >>indeed redundant, as the driver developer can easily check its
> >>origin during implementation.
> >
> >Fine for me.
> 
> Fine regarding my explanation or you agree that it is redundant?

:-) With the explanation. I might have removed it but I'm having it there as
well.

-- 
Regards,

Sakari Ailus
e-mail: sakari.ai...@iki.fi XMPP: sai...@retiisi.org.uk
--
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/RFC v9 06/19] DT: Add documentation for the mfd Maxim max77693

2014-12-09 Thread Sakari Ailus
Hi Jacek,

On Thu, Dec 04, 2014 at 12:40:48PM +0100, Jacek Anaszewski wrote:
> >>+   the flash/torch.
> >>+- maxim,trigger : Array of flags indicating which trigger can activate 
> >>given led
> >>+   in order: fled1, fled2.
> >>+   Possible flag values (can be combined):
> >>+   MAX77693_LED_TRIG_FLASHEN - FLASHEN pin of the chip,
> >>+   MAX77693_LED_TRIG_TORCHEN - TORCHEN pin of the chip,
> >>+   MAX77693_LED_TRIG_SOFTWARE - software via I2C command.
> >
> >Is there a need to prevent strobing using a certain method? Just wondering.
> 
> In some cases it could be convenient to prevent some options through
> device tree.

Do you have that need now?

If not, I'd propose to postpone this and add it only if there ever is one.

-- 
Kind regards,

Sakari Ailus
e-mail: sakari.ai...@iki.fi XMPP: sai...@retiisi.org.uk
--
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/RFC v9 04/19] mfd: max77693: adjust max77693_led_platform_data

2014-12-09 Thread Jacek Anaszewski

On 12/09/2014 02:50 PM, Lee Jones wrote:

On Tue, 09 Dec 2014, Jacek Anaszewski wrote:


On 12/09/2014 11:04 AM, Lee Jones wrote:

On Tue, 09 Dec 2014, Jacek Anaszewski wrote:


On 12/09/2014 09:50 AM, Lee Jones wrote:

On Wed, 03 Dec 2014, Jacek Anaszewski wrote:


Add "label" array for Device Tree strings with the name of a LED device
and make flash_timeout a two element array, for caching the sub-led
related flash timeout. Added is also an array for caching pointers to the
sub-nodes representing sub-leds.

Signed-off-by: Jacek Anaszewski 
Acked-by: Kyungmin Park 
Cc: Chanwoo Choi 
Cc: Lee Jones 
---
  include/linux/mfd/max77693.h |4 +++-
  1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/include/linux/mfd/max77693.h b/include/linux/mfd/max77693.h
index f0b6585..c80ee99 100644
--- a/include/linux/mfd/max77693.h
+++ b/include/linux/mfd/max77693.h
@@ -88,16 +88,18 @@ enum max77693_led_boost_mode {
  };

  struct max77693_led_platform_data {
+   const char *label[2];
u32 fleds[2];
u32 iout_torch[2];for_each_available_child_of_node
u32 iout_flash[2];
u32 trigger[2];
u32 trigger_type[2];
+   u32 flash_timeout[2];
u32 num_leds;
u32 boost_mode;
-   u32 flash_timeout;
u32 boost_vout;
u32 low_vsys;
+   struct device_node *sub_nodes[2];


I haven't seen anyone do this before.  Why can't you use the provided
OF functions to traverse through your tree?


I use for_each_available_child_of_node when parsing DT node, but I
need to cache the pointer to sub-node to be able to use it later
when it needs to be passed to V4L2 sub-device which is then
asynchronously matched by the phandle to sub-node.

If it is not well seen to cache it in the platform data then
I will find different way to accomplish this.


I haven't seen the end-driver for this, but why can't you use that
device's of_node pointer?


Maybe it is indeed a good idea. I could pass the of_node pointer
and the sub-led identifier to the V4L2 sub-device and there look
for the sub-node containing relevant identifier. The downside
would be only that for_each_available_child_of_node would
have to be called twice - in the led driver and in the V4L2 sub-device.
I think that we can live with it.


Are the LED and V4L2 drivers children of this MFD?  If so, you can use
the of_compatible attribute in struct mfd_cell to populate the each
child's of_node dynamically i.e. the MFD core will do that for you.



V4L2 driver wraps LED driver. This way the LED device can be
controlled with use of two interfaces - LED subsystem sysfs
and V4L2 Flash. This is the aim of the whole patch set.

I've thought it over again and it seems that I will need to cache
somewhere these sub_nodes pointers. They have to be easily accessible
for the V4L2 sub-device as it can be asynchronously registered
or unregistered within V4L2 media device. Sub-devices are matched
basing on the sub-node phandle.

Best Regards,
Jacek Anaszewski
--
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/RFC v9 04/19] mfd: max77693: adjust max77693_led_platform_data

2014-12-09 Thread Lee Jones
On Tue, 09 Dec 2014, Jacek Anaszewski wrote:

> On 12/09/2014 11:04 AM, Lee Jones wrote:
> >On Tue, 09 Dec 2014, Jacek Anaszewski wrote:
> >
> >>On 12/09/2014 09:50 AM, Lee Jones wrote:
> >>>On Wed, 03 Dec 2014, Jacek Anaszewski wrote:
> >>>
> Add "label" array for Device Tree strings with the name of a LED device
> and make flash_timeout a two element array, for caching the sub-led
> related flash timeout. Added is also an array for caching pointers to the
> sub-nodes representing sub-leds.
> 
> Signed-off-by: Jacek Anaszewski 
> Acked-by: Kyungmin Park 
> Cc: Chanwoo Choi 
> Cc: Lee Jones 
> ---
>   include/linux/mfd/max77693.h |4 +++-
>   1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/include/linux/mfd/max77693.h b/include/linux/mfd/max77693.h
> index f0b6585..c80ee99 100644
> --- a/include/linux/mfd/max77693.h
> +++ b/include/linux/mfd/max77693.h
> @@ -88,16 +88,18 @@ enum max77693_led_boost_mode {
>   };
> 
>   struct max77693_led_platform_data {
> + const char *label[2];
>   u32 fleds[2];
>   u32 iout_torch[2];for_each_available_child_of_node
>   u32 iout_flash[2];
>   u32 trigger[2];
>   u32 trigger_type[2];
> + u32 flash_timeout[2];
>   u32 num_leds;
>   u32 boost_mode;
> - u32 flash_timeout;
>   u32 boost_vout;
>   u32 low_vsys;
> + struct device_node *sub_nodes[2];
> >>>
> >>>I haven't seen anyone do this before.  Why can't you use the provided
> >>>OF functions to traverse through your tree?
> >>
> >>I use for_each_available_child_of_node when parsing DT node, but I
> >>need to cache the pointer to sub-node to be able to use it later
> >>when it needs to be passed to V4L2 sub-device which is then
> >>asynchronously matched by the phandle to sub-node.
> >>
> >>If it is not well seen to cache it in the platform data then
> >>I will find different way to accomplish this.
> >
> >I haven't seen the end-driver for this, but why can't you use that
> >device's of_node pointer?
> 
> Maybe it is indeed a good idea. I could pass the of_node pointer
> and the sub-led identifier to the V4L2 sub-device and there look
> for the sub-node containing relevant identifier. The downside
> would be only that for_each_available_child_of_node would
> have to be called twice - in the led driver and in the V4L2 sub-device.
> I think that we can live with it.

Are the LED and V4L2 drivers children of this MFD?  If so, you can use
the of_compatible attribute in struct mfd_cell to populate the each
child's of_node dynamically i.e. the MFD core will do that for you.

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
--
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/RFC v9 05/19] leds: Add support for max77693 mfd flash cell

2014-12-09 Thread Sakari Ailus
Hi Jacek,

On Thu, Dec 04, 2014 at 12:06:59PM +0100, Jacek Anaszewski wrote:
> Hi Sakari,
> 
> Thanks for the review.

You're welcome! :-)

> On 12/04/2014 10:39 AM, Sakari Ailus wrote:
> >Hi Jacek,
> >
> >On Wed, Dec 03, 2014 at 05:06:40PM +0100, Jacek Anaszewski wrote:
> >>This patch adds led-flash support to Maxim max77693 chipset.
> >>A device can be exposed to user space through LED subsystem
> >>sysfs interface. Device supports up to two leds which can
> >>work in flash and torch mode. The leds can be triggered
> >>externally or by software.
> >>
> >>Signed-off-by: Jacek Anaszewski 
> >>Signed-off-by: Andrzej Hajda 
> >>Acked-by: Kyungmin Park 
> >>Cc: Bryan Wu 
> >>Cc: Richard Purdie 
> >>Cc: Lee Jones 
> >>Cc: Chanwoo Choi 
> >>---
> >>  drivers/leds/Kconfig |   10 +
> >>  drivers/leds/Makefile|1 +
> >>  drivers/leds/leds-max77693.c | 1023 
> >> ++
> >>  3 files changed, 1034 insertions(+)
> >>  create mode 100644 drivers/leds/leds-max77693.c
> >>
> >>diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig
> >>index fa8021e..2e66d55 100644
> >>--- a/drivers/leds/Kconfig
> >>+++ b/drivers/leds/Kconfig
> >>@@ -463,6 +463,16 @@ config LEDS_TCA6507
> >>  LED driver chips accessed via the I2C bus.
> >>  Driver support brightness control and hardware-assisted blinking.
> >>
> >>+config LEDS_MAX77693
> >>+   tristate "LED support for MAX77693 Flash"
> >>+   depends on LEDS_CLASS_FLASH
> >>+   depends on MFD_MAX77693
> >>+   depends on OF
> >>+   help
> >>+ This option enables support for the flash part of the MAX77693
> >>+ multifunction device. It has build in control for two leds in flash
> >>+ and torch mode.
> >>+
> >>  config LEDS_MAX8997
> >>tristate "LED support for MAX8997 PMIC"
> >>depends on LEDS_CLASS && MFD_MAX8997
> >>diff --git a/drivers/leds/Makefile b/drivers/leds/Makefile
> >>index cbba921..57ca62b 100644
> >>--- a/drivers/leds/Makefile
> >>+++ b/drivers/leds/Makefile
> >>@@ -52,6 +52,7 @@ obj-$(CONFIG_LEDS_MC13783)+= 
> >>leds-mc13783.o
> >>  obj-$(CONFIG_LEDS_NS2)+= leds-ns2.o
> >>  obj-$(CONFIG_LEDS_NETXBIG)+= leds-netxbig.o
> >>  obj-$(CONFIG_LEDS_ASIC3)  += leds-asic3.o
> >>+obj-$(CONFIG_LEDS_MAX77693)+= leds-max77693.o
> >>  obj-$(CONFIG_LEDS_MAX8997)+= leds-max8997.o
> >>  obj-$(CONFIG_LEDS_LM355x) += leds-lm355x.o
> >>  obj-$(CONFIG_LEDS_BLINKM) += leds-blinkm.o
> >>diff --git a/drivers/leds/leds-max77693.c b/drivers/leds/leds-max77693.c
> >>new file mode 100644
> >>index 000..67a2f8f
> >>--- /dev/null
> >>+++ b/drivers/leds/leds-max77693.c
> >>@@ -0,0 +1,1023 @@
> >>+/*
> >>+ * LED Flash class driver for the flash cell of max77693 mfd.
> >>+ *
> >>+ * Copyright (C) 2014, Samsung Electronics Co., Ltd.
> >>+ *
> >>+ * Authors: Jacek Anaszewski 
> >>+ *  Andrzej Hajda 
> >>+ *
> >>+ * 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.
> >>+ */
> >>+
> >>+#include 
> >>+#include 
> >>+#include 
> >>+#include 
> >>+#include 
> >>+#include 
> >>+#include 
> >>+#include 
> >>+#include 
> >>+#include 
> >>+
> >>+#define MODE_OFF   0
> >>+#define MODE_FLASH1(1 << 0)
> >>+#define MODE_FLASH2(1 << 1)
> >>+#define MODE_TORCH1(1 << 2)
> >>+#define MODE_TORCH2(1 << 3)
> >>+#define MODE_FLASH_EXTERNAL1   (1 << 4)
> >>+#define MODE_FLASH_EXTERNAL2   (1 << 5)
> >
> >You could do this based on an argument (led number). E.g.
> >
> >#define MODE_FLASH_EXTERNAL(a)   (1 << (4 + a))
> 
> OK.
> 
> >>+#define MODE_FLASH (MODE_FLASH1 | MODE_FLASH2 | \
> >>+MODE_FLASH_EXTERNAL1 | MODE_FLASH_EXTERNAL2)
> >>+
> >>+#define FLED1_IOUT (1 << 0)
> >>+#define FLED2_IOUT (1 << 1)
> >>+
> >>+enum {
> >>+   FLED1,
> >>+   FLED2
> >>+};
> >>+
> >>+enum {
> >>+   FLASH,
> >>+   TORCH
> >>+};
> >>+
> >>+struct max77693_sub_led {

This could then be renamed as "max77693_led"; up to you.

> >>+   struct led_classdev_flash ldev;
> >>+   struct work_struct work_brightness_set;
> >>+
> >>+   unsigned int torch_brightness;
> >>+   unsigned int flash_timeout;
> >>+};
> >>+
> >>+struct max77693_led {
> >
> >As this does not refer to a device, how about struct max77693_device, for
> >instance?
> 
> OK.
> 
> >>+   struct regmap *regmap;
> >>+   struct platform_device *pdev;
> >>+   struct max77693_led_platform_data *pdata;
> >>+   struct mutex lock;
> >>+
> >>+   struct max77693_sub_led sub_leds[2];
> >>+
> >>+   unsigned int current_flash_timeout;
> >>+   unsigned int mode_flags;
> >>+   u8 torch_iout_reg;
> >>+   bool iout_joint;
> >>+   int strobing_sub_led_id;
> >>+};
> >>+
> >>+struct max77693_led_settings {
> >>+   struct led_flash_setting torc

Re: [PATCH/RFC v9 02/19] Documentation: leds: Add description of LED Flash class extension

2014-12-09 Thread Jacek Anaszewski

Hi Sakari,

On 12/09/2014 01:38 PM, Sakari Ailus wrote:

[...]


How does the user btw. figure out which flash LEDs may be strobed
synchronously using the LED flash interface?


The flash_sync_strobe argument is absent if synchronized strobe
is not available for a LED. The driver defines this by setting
newly added LED_DEV_CAP_COMPOUND flag.


I meant that how does the user figure out which LEDs may be strobed
synchronously, together. Say, if you have two of these chips and four LEDs,
then how does it work? :-)



User can figure it out by checking the existence of the
flash_sync_strobe attribute. Sub-leds can by synchronized only
when are driven by common chip. It is assumed that sub-leds of
one chip will have common segment in their name, defined in
DT 'label' property. Maybe we should enforce it by adding another
property to the leds/common.txt DT binding, e.g. 'device-prefix'?

Best Regards,
Jacek Anaszewski

--
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/RFC v9 01/19] leds: Add LED Flash class extension to the LED subsystem

2014-12-09 Thread Jacek Anaszewski

Hi Sakari,

On 12/09/2014 01:36 PM, Sakari Ailus wrote:

Hi Jacek,

On Thu, Dec 04, 2014 at 10:29:10AM +0100, Jacek Anaszewski wrote:
...

+static struct attribute *led_flash_strobe_attrs[] = {
+   &dev_attr_flash_strobe.attr,
+   NULL,
+};
+
+static struct attribute *led_flash_timeout_attrs[] = {
+   &dev_attr_flash_timeout.attr,
+   &dev_attr_max_flash_timeout.attr,
+   NULL,
+};
+
+static struct attribute *led_flash_brightness_attrs[] = {
+   &dev_attr_flash_brightness.attr,
+   &dev_attr_max_flash_brightness.attr,
+   NULL,
+};
+
+static struct attribute *led_flash_fault_attrs[] = {
+   &dev_attr_flash_fault.attr,
+   NULL,
+};
+
+static struct attribute *led_flash_sync_strobe_attrs[] = {
+   &dev_attr_flash_sync_strobe.attr,
+   NULL,
+};
+
+static const struct attribute_group led_flash_strobe_group = {
+   .attrs = led_flash_strobe_attrs,
+};
+
+static const struct attribute_group led_flash_timeout_group = {
+   .attrs = led_flash_timeout_attrs,
+};
+
+static const struct attribute_group led_flash_brightness_group = {
+   .attrs = led_flash_brightness_attrs,
+};
+
+static const struct attribute_group led_flash_fault_group = {
+   .attrs = led_flash_fault_attrs,
+};
+
+static const struct attribute_group led_flash_sync_strobe_group = {
+   .attrs = led_flash_sync_strobe_attrs,
+};
+
+static const struct attribute_group *flash_groups[] = {
+   &led_flash_strobe_group,
+   NULL,
+   NULL,
+   NULL,
+   NULL,
+   NULL,
+   NULL
+};
+
+static void led_flash_resume(struct led_classdev *led_cdev)
+{
+   struct led_classdev_flash *flash = lcdev_to_flash(led_cdev);
+
+   call_flash_op(flash, flash_brightness_set, flash->brightness.val);
+   call_flash_op(flash, timeout_set, flash->timeout.val);
+}
+
+static void led_flash_init_sysfs_groups(struct led_classdev_flash *flash)
+{
+   struct led_classdev *led_cdev = &flash->led_cdev;
+   const struct led_flash_ops *ops = flash->ops;
+   int num_sysfs_groups = 1;
+
+   if (ops->flash_brightness_set)
+   flash_groups[num_sysfs_groups++] = &led_flash_brightness_group;
+
+   if (ops->timeout_set)
+   flash_groups[num_sysfs_groups++] = &led_flash_timeout_group;
+
+   if (ops->fault_get)
+   flash_groups[num_sysfs_groups++] = &led_flash_fault_group;
+
+   if (led_cdev->flags & LED_DEV_CAP_COMPOUND)
+   flash_groups[num_sysfs_groups++] = &led_flash_sync_strobe_group;
+
+   led_cdev->groups = flash_groups;


Shouldn't you have groups local to the device instead? If you register
another flash device bad things will happen if the ops the device supports
are different.


The groups are local to the device. A LED class device is registered
with device_create_with_groups called from led_classdev_register
function. It is passed led_cdev->groups in the fifth argument.


The groups pointer will be stored in struct device. If you have another
driver using different groups, it will affect the groups for all flash
devices that use the same groups pointer. I'm not sure what exactly would
follow from that but I'd rather not change them once the device is created.


I had to take another look at this to understand the problem.
I think that the best option will be making flash_groups array
a member of struct led_classdev_flash.


+}
+
+int led_classdev_flash_register(struct device *parent,
+   struct led_classdev_flash *flash)
+{
+   struct led_classdev *led_cdev;
+   const struct led_flash_ops *ops;
+   int ret;
+
+   if (!flash)


Do you have a use case for this?


This is just a guard against NULL pointer dereference. Maybe it is
indeed redundant, as the driver developer can easily check its
origin during implementation.


Fine for me.


Fine regarding my explanation or you agree that it is redundant?

Best Regards,
Jacek Anaszewski

--
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/RFC v9 02/19] Documentation: leds: Add description of LED Flash class extension

2014-12-09 Thread Sakari Ailus
Hi Jacek,

On Thu, Dec 04, 2014 at 10:42:55AM +0100, Jacek Anaszewski wrote:
> Hi Sakari,
> 
> Thanks for the review.
> 
> On 12/03/2014 06:08 PM, Sakari Ailus wrote:
> >Hi Jacek,
> >
> >On Wed, Dec 03, 2014 at 05:06:37PM +0100, Jacek Anaszewski wrote:
> >>The documentation being added contains overall description of the
> >>LED Flash Class and the related sysfs attributes.
> >>
> >>Signed-off-by: Jacek Anaszewski 
> >>Acked-by: Kyungmin Park 
> >>Cc: Bryan Wu 
> >>Cc: Richard Purdie 
> >>---
> >>  Documentation/leds/leds-class-flash.txt |   50 
> >> +++
> >>  1 file changed, 50 insertions(+)
> >>  create mode 100644 Documentation/leds/leds-class-flash.txt
> >>
> >>diff --git a/Documentation/leds/leds-class-flash.txt 
> >>b/Documentation/leds/leds-class-flash.txt
> >>new file mode 100644
> >>index 000..82e58b1
> >>--- /dev/null
> >>+++ b/Documentation/leds/leds-class-flash.txt
> >>@@ -0,0 +1,50 @@
> >>+
> >>+Flash LED handling under Linux
> >>+==
> >>+
> >>+Some LED devices support two modes - torch and flash. The modes are
> >>+supported by the LED class (see Documentation/leds/leds-class.txt)
> >>+and LED Flash class respectively.
> >>+
> >>+In order to enable support for flash LEDs CONFIG_LEDS_CLASS_FLASH symbol
> >>+must be defined in the kernel config. A flash LED driver must register
> >>+in the LED subsystem with led_classdev_flash_register to gain flash
> >>+capabilities.
> >>+
> >>+Following sysfs attributes are exposed for controlling flash led devices:
> >>+
> >>+   - flash_brightness - flash LED brightness in microamperes (RW)
> >>+   - max_flash_brightness - maximum available flash LED brightness (RO)
> >>+   - flash_timeout - flash strobe duration in microseconds (RW)
> >>+   - max_flash_timeout - maximum available flash strobe duration (RO)
> >>+   - flash_strobe - flash strobe state (RW)
> >>+   - flash_sync_strobe - one flash device can control more than one
> >>+ sub-led; when this atrribute is set to 1
> >
> >s/atrribute/attribute/
> >
> >>+ the flash led will be strobed synchronously
> >>+ with the other one controlled by the same
> >>+ device; flash timeout setting is inherited
> >>+ from the led being strobed explicitly and
> >>+ flash brightness setting of a sub-led's
> >>+ being synchronized is used (RW)
> >
> >The flash brightness shouldn't be determined by the strobed LED. If this is
> >a property of the hardware, then be it, but in general no, it it shouldn't
> >be an interface requirement. I think this should just say that the strobe is
> >synchronised.
> 
> I intended this to sound exactly as you laid it out above, but maybe it
> is obscure English. "and flash brightness setting of a sub-led >>>being
> synchronized<<< is used" - from my point of view the led being
> synchronized is the one that isn't strobed explicitly. But I'm ok with
> confining ourselves only to saying that strobe is synchronized.

Agreed.

> >How does the user btw. figure out which flash LEDs may be strobed
> >synchronously using the LED flash interface?
> 
> The flash_sync_strobe argument is absent if synchronized strobe
> is not available for a LED. The driver defines this by setting
> newly added LED_DEV_CAP_COMPOUND flag.

I meant that how does the user figure out which LEDs may be strobed
synchronously, together. Say, if you have two of these chips and four LEDs,
then how does it work? :-)

-- 
Kind regards,

Sakari Ailus
e-mail: sakari.ai...@iki.fi XMPP: sai...@retiisi.org.uk
--
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/RFC v9 01/19] leds: Add LED Flash class extension to the LED subsystem

2014-12-09 Thread Sakari Ailus
Hi Jacek,

On Thu, Dec 04, 2014 at 10:29:10AM +0100, Jacek Anaszewski wrote:
...
> >>+static struct attribute *led_flash_strobe_attrs[] = {
> >>+   &dev_attr_flash_strobe.attr,
> >>+   NULL,
> >>+};
> >>+
> >>+static struct attribute *led_flash_timeout_attrs[] = {
> >>+   &dev_attr_flash_timeout.attr,
> >>+   &dev_attr_max_flash_timeout.attr,
> >>+   NULL,
> >>+};
> >>+
> >>+static struct attribute *led_flash_brightness_attrs[] = {
> >>+   &dev_attr_flash_brightness.attr,
> >>+   &dev_attr_max_flash_brightness.attr,
> >>+   NULL,
> >>+};
> >>+
> >>+static struct attribute *led_flash_fault_attrs[] = {
> >>+   &dev_attr_flash_fault.attr,
> >>+   NULL,
> >>+};
> >>+
> >>+static struct attribute *led_flash_sync_strobe_attrs[] = {
> >>+   &dev_attr_flash_sync_strobe.attr,
> >>+   NULL,
> >>+};
> >>+
> >>+static const struct attribute_group led_flash_strobe_group = {
> >>+   .attrs = led_flash_strobe_attrs,
> >>+};
> >>+
> >>+static const struct attribute_group led_flash_timeout_group = {
> >>+   .attrs = led_flash_timeout_attrs,
> >>+};
> >>+
> >>+static const struct attribute_group led_flash_brightness_group = {
> >>+   .attrs = led_flash_brightness_attrs,
> >>+};
> >>+
> >>+static const struct attribute_group led_flash_fault_group = {
> >>+   .attrs = led_flash_fault_attrs,
> >>+};
> >>+
> >>+static const struct attribute_group led_flash_sync_strobe_group = {
> >>+   .attrs = led_flash_sync_strobe_attrs,
> >>+};
> >>+
> >>+static const struct attribute_group *flash_groups[] = {
> >>+   &led_flash_strobe_group,
> >>+   NULL,
> >>+   NULL,
> >>+   NULL,
> >>+   NULL,
> >>+   NULL,
> >>+   NULL
> >>+};
> >>+
> >>+static void led_flash_resume(struct led_classdev *led_cdev)
> >>+{
> >>+   struct led_classdev_flash *flash = lcdev_to_flash(led_cdev);
> >>+
> >>+   call_flash_op(flash, flash_brightness_set, flash->brightness.val);
> >>+   call_flash_op(flash, timeout_set, flash->timeout.val);
> >>+}
> >>+
> >>+static void led_flash_init_sysfs_groups(struct led_classdev_flash *flash)
> >>+{
> >>+   struct led_classdev *led_cdev = &flash->led_cdev;
> >>+   const struct led_flash_ops *ops = flash->ops;
> >>+   int num_sysfs_groups = 1;
> >>+
> >>+   if (ops->flash_brightness_set)
> >>+   flash_groups[num_sysfs_groups++] = &led_flash_brightness_group;
> >>+
> >>+   if (ops->timeout_set)
> >>+   flash_groups[num_sysfs_groups++] = &led_flash_timeout_group;
> >>+
> >>+   if (ops->fault_get)
> >>+   flash_groups[num_sysfs_groups++] = &led_flash_fault_group;
> >>+
> >>+   if (led_cdev->flags & LED_DEV_CAP_COMPOUND)
> >>+   flash_groups[num_sysfs_groups++] = &led_flash_sync_strobe_group;
> >>+
> >>+   led_cdev->groups = flash_groups;
> >
> >Shouldn't you have groups local to the device instead? If you register
> >another flash device bad things will happen if the ops the device supports
> >are different.
> 
> The groups are local to the device. A LED class device is registered
> with device_create_with_groups called from led_classdev_register
> function. It is passed led_cdev->groups in the fifth argument.

The groups pointer will be stored in struct device. If you have another
driver using different groups, it will affect the groups for all flash
devices that use the same groups pointer. I'm not sure what exactly would
follow from that but I'd rather not change them once the device is created.

> >>+}
> >>+
> >>+int led_classdev_flash_register(struct device *parent,
> >>+   struct led_classdev_flash *flash)
> >>+{
> >>+   struct led_classdev *led_cdev;
> >>+   const struct led_flash_ops *ops;
> >>+   int ret;
> >>+
> >>+   if (!flash)
> >
> >Do you have a use case for this?
> 
> This is just a guard against NULL pointer dereference. Maybe it is
> indeed redundant, as the driver developer can easily check its
> origin during implementation.

Fine for me.

-- 
Regards,

Sakari Ailus
e-mail: sakari.ai...@iki.fi XMPP: sai...@retiisi.org.uk
--
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] v4l2-ctl: Remove file entry from Android.mk

2014-12-09 Thread matrandg
From: Mats Randgaard 

v4l2-ctl-test-patterns.cpp is removed, but still listed in Android.mk.
---
 utils/v4l2-ctl/Android.mk |5 ++---
 1 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/utils/v4l2-ctl/Android.mk b/utils/v4l2-ctl/Android.mk
index bbd151a..2f0e5b4 100644
--- a/utils/v4l2-ctl/Android.mk
+++ b/utils/v4l2-ctl/Android.mk
@@ -19,7 +19,6 @@ LOCAL_SRC_FILES := \
 v4l2-ctl.cpp v4l2-ctl.h v4l2-ctl-common.cpp v4l2-ctl-tuner.cpp \
 v4l2-ctl-io.cpp v4l2-ctl-stds.cpp v4l2-ctl-vidcap.cpp v4l2-ctl-vidout.cpp \
 v4l2-ctl-overlay.cpp v4l2-ctl-vbi.cpp v4l2-ctl-selection.cpp 
v4l2-ctl-misc.cpp \
-v4l2-ctl-streaming.cpp v4l2-ctl-test-patterns.cpp v4l2-ctl-sdr.cpp \
-v4l2-ctl-edid.cpp vivid-tpg-colors.c vivid-tpg.c
-
+v4l2-ctl-streaming.cpp v4l2-ctl-sdr.cpp v4l2-ctl-edid.cpp 
vivid-tpg-colors.c \
+vivid-tpg.c
 include $(BUILD_EXECUTABLE)
-- 
1.7.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 v5] media: platform: add VPFE capture driver support for AM437X

2014-12-09 Thread Hans Verkuil
On 12/08/14 23:21, Lad, Prabhakar wrote:
> From: Benoit Parrot 
> 
> This patch adds Video Processing Front End (VPFE) driver for
> AM437X family of devices
> Driver supports the following:
> - V4L2 API using MMAP buffer access based on videobuf2 api
> - Asynchronous sensor/decoder sub device registration
> - DT support
> 
> Signed-off-by: Benoit Parrot 
> Signed-off-by: Darren Etheridge 
> Signed-off-by: Lad, Prabhakar 
> ---
>  Changes for v5:
>  1: Fixed review comments pointed out by Hans, fixing race condition.
> 
>  v4l2-compliance output:

Thanks!



>  --
> 
> diff --git a/drivers/media/platform/am437x/am437x-vpfe.c 
> b/drivers/media/platform/am437x/am437x-vpfe.c
> new file mode 100644
> index 000..c2b29a2
> --- /dev/null
> +++ b/drivers/media/platform/am437x/am437x-vpfe.c
> +/*
> + * vpfe_release : This function is based on the vb2_fop_release
> + * helper function.
> + * It has been augmented to handle module power management,
> + * by disabling/enabling h/w module fcntl clock when necessary.
> + */
> +static int vpfe_release(struct file *file)
> +{
> + struct vpfe_device *vpfe = video_drvdata(file);
> + bool close = v4l2_fh_is_singular_file(file);
> + int ret;
> +
> + vpfe_dbg(2, vpfe, "vpfe_release\n");
> +
> + mutex_lock(&vpfe->lock);

The v4l2_fh_is_singular_file() call should be inside the lock as well.
So:

close = v4l2_fh_is_singular_file(file);

> +
> + ret = _vb2_fop_release(file, NULL);
> + if (close)
> + vpfe_ccdc_close(&vpfe->ccdc, vpfe->pdev);
> +
> + mutex_unlock(&vpfe->lock);
> +
> + return ret;
> +}
> +
> +/*
> + * vpfe_open : This function is based on the v4l2_fh_open helper function.
> + * It has been augmented to handle module power management,
> + * by disabling/enabling h/w module fcntl clock when necessary.
> + */
> +static int vpfe_open(struct file *file)
> +{
> + struct vpfe_device *vpfe = video_drvdata(file);
> + int ret;
> +
> + ret = v4l2_fh_open(file);

Same here, v4l2_fh_open should be inside the lock.

> + if (ret) {
> + vpfe_err(vpfe, "v4l2_fh_open failed\n");
> + return ret;
> + }
> +
> + mutex_lock(&vpfe->lock);

So:

ret = v4l2_fh_open(file);
if (ret) {
vpfe_err(vpfe, "v4l2_fh_open failed\n");
goto unlock;
}


> +
> + if (!v4l2_fh_is_singular_file(file))
> + goto unlock;
> +
> + if (vpfe_initialize_device(vpfe)) {
> + v4l2_fh_release(file);
> + ret = -ENODEV;
> + }
> +
> +unlock:
> + mutex_unlock(&vpfe->lock);
> + return ret;
> +}

Regards,

Hans
--
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/RFC v9 04/19] mfd: max77693: adjust max77693_led_platform_data

2014-12-09 Thread Jacek Anaszewski

On 12/09/2014 11:04 AM, Lee Jones wrote:

On Tue, 09 Dec 2014, Jacek Anaszewski wrote:


On 12/09/2014 09:50 AM, Lee Jones wrote:

On Wed, 03 Dec 2014, Jacek Anaszewski wrote:


Add "label" array for Device Tree strings with the name of a LED device
and make flash_timeout a two element array, for caching the sub-led
related flash timeout. Added is also an array for caching pointers to the
sub-nodes representing sub-leds.

Signed-off-by: Jacek Anaszewski 
Acked-by: Kyungmin Park 
Cc: Chanwoo Choi 
Cc: Lee Jones 
---
  include/linux/mfd/max77693.h |4 +++-
  1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/include/linux/mfd/max77693.h b/include/linux/mfd/max77693.h
index f0b6585..c80ee99 100644
--- a/include/linux/mfd/max77693.h
+++ b/include/linux/mfd/max77693.h
@@ -88,16 +88,18 @@ enum max77693_led_boost_mode {
  };

  struct max77693_led_platform_data {
+   const char *label[2];
u32 fleds[2];
u32 iout_torch[2];for_each_available_child_of_node
u32 iout_flash[2];
u32 trigger[2];
u32 trigger_type[2];
+   u32 flash_timeout[2];
u32 num_leds;
u32 boost_mode;
-   u32 flash_timeout;
u32 boost_vout;
u32 low_vsys;
+   struct device_node *sub_nodes[2];


I haven't seen anyone do this before.  Why can't you use the provided
OF functions to traverse through your tree?


I use for_each_available_child_of_node when parsing DT node, but I
need to cache the pointer to sub-node to be able to use it later
when it needs to be passed to V4L2 sub-device which is then
asynchronously matched by the phandle to sub-node.

If it is not well seen to cache it in the platform data then
I will find different way to accomplish this.


I haven't seen the end-driver for this, but why can't you use that
device's of_node pointer?


Maybe it is indeed a good idea. I could pass the of_node pointer
and the sub-led identifier to the V4L2 sub-device and there look
for the sub-node containing relevant identifier. The downside
would be only that for_each_available_child_of_node would
have to be called twice - in the led driver and in the V4L2 sub-device.
I think that we can live with it.

Thanks for the hint.

Best Regards,
Jacek Anaszewski
--
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/RFC v9 04/19] mfd: max77693: adjust max77693_led_platform_data

2014-12-09 Thread Lee Jones
On Tue, 09 Dec 2014, Jacek Anaszewski wrote:

> On 12/09/2014 09:50 AM, Lee Jones wrote:
> >On Wed, 03 Dec 2014, Jacek Anaszewski wrote:
> >
> >>Add "label" array for Device Tree strings with the name of a LED device
> >>and make flash_timeout a two element array, for caching the sub-led
> >>related flash timeout. Added is also an array for caching pointers to the
> >>sub-nodes representing sub-leds.
> >>
> >>Signed-off-by: Jacek Anaszewski 
> >>Acked-by: Kyungmin Park 
> >>Cc: Chanwoo Choi 
> >>Cc: Lee Jones 
> >>---
> >>  include/linux/mfd/max77693.h |4 +++-
> >>  1 file changed, 3 insertions(+), 1 deletion(-)
> >>
> >>diff --git a/include/linux/mfd/max77693.h b/include/linux/mfd/max77693.h
> >>index f0b6585..c80ee99 100644
> >>--- a/include/linux/mfd/max77693.h
> >>+++ b/include/linux/mfd/max77693.h
> >>@@ -88,16 +88,18 @@ enum max77693_led_boost_mode {
> >>  };
> >>
> >>  struct max77693_led_platform_data {
> >>+   const char *label[2];
> >>u32 fleds[2];
> >>u32 iout_torch[2];for_each_available_child_of_node
> >>u32 iout_flash[2];
> >>u32 trigger[2];
> >>u32 trigger_type[2];
> >>+   u32 flash_timeout[2];
> >>u32 num_leds;
> >>u32 boost_mode;
> >>-   u32 flash_timeout;
> >>u32 boost_vout;
> >>u32 low_vsys;
> >>+   struct device_node *sub_nodes[2];
> >
> >I haven't seen anyone do this before.  Why can't you use the provided
> >OF functions to traverse through your tree?
> 
> I use for_each_available_child_of_node when parsing DT node, but I
> need to cache the pointer to sub-node to be able to use it later
> when it needs to be passed to V4L2 sub-device which is then
> asynchronously matched by the phandle to sub-node.
> 
> If it is not well seen to cache it in the platform data then
> I will find different way to accomplish this.

I haven't seen the end-driver for this, but why can't you use that
device's of_node pointer?

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
--
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/RFC v9 03/19] mfd: max77693: Modify flash cell name identifiers

2014-12-09 Thread Lee Jones
On Tue, 09 Dec 2014, Jacek Anaszewski wrote:

> On 12/09/2014 09:52 AM, Lee Jones wrote:
> >On Wed, 03 Dec 2014, Jacek Anaszewski wrote:
> >
> >>Change flash cell identifiers from max77693-flash to max77693-led
> >>to avoid confusion with NOR/NAND Flash.
> >>
> >>Signed-off-by: Jacek Anaszewski 
> >>Acked-by: Kyungmin Park 
> >>Cc: Chanwoo Choi 
> >>Cc: Lee Jones 
> >>---
> >>  drivers/mfd/max77693.c |4 ++--
> >>  1 file changed, 2 insertions(+), 2 deletions(-)
> >>
> >>diff --git a/drivers/mfd/max77693.c b/drivers/mfd/max77693.c
> >>index a159593..cb14afa 100644
> >>--- a/drivers/mfd/max77693.c
> >>+++ b/drivers/mfd/max77693.c
> >>@@ -53,8 +53,8 @@ static const struct mfd_cell max77693_devs[] = {
> >>.of_compatible = "maxim,max77693-haptic",
> >>},
> >>{
> >>-   .name = "max77693-flash",
> >>-   .of_compatible = "maxim,max77693-flash",
> >>+   .name = "max77693-led",
> >>+   .of_compatible = "maxim,max77693-led",
> >
> >This is fine by me, so long as you've been through the usual
> >deprecation procedures or this platform is still WiP.
> 
> It was me who added of_compatible for max77693-flash, but the
> related led driver has not been yet merged and there are no
> other drivers depending on it.

Very well.

For my own reference:

Acked-by: Lee Jones 

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
--
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


[GIT PULL FOR v3.19] More 3.19 fixes

2014-12-09 Thread Hans Verkuil
One vivid typo, two patches that fix the cx88 driver and one patch that
warns if a driver forgets to set device_caps in VIDIOC_QUERYCAP. All
drivers are now supposed to do that, but it is too easy to forget.

I found at least one case where that didn't happen, so expect to see at
least one more pull request for 3.19 fixing this.

Regards,

Hans

The following changes since commit 71947828caef0c83d4245f7d1eaddc799b4ff1d1:

  [media] mn88473: One function call less in mn88473_init() after error 
(2014-12-04 16:00:47 -0200)

are available in the git repository at:

  git://linuxtv.org/hverkuil/media_tree.git for-v3.19n

for you to fetch changes up to d61e832500336d6c1a0267c9b1ed6613156d9fde:

  cx88: remove leftover start_video_dma() call (2014-12-09 10:48:34 +0100)


Hans Verkuil (4):
  vivid: fix CROP_BOUNDS typo for video output
  v4l2-ioctl: WARN_ON if querycap didn't fill device_caps
  cx88: add missing alloc_ctx support
  cx88: remove leftover start_video_dma() call

 drivers/media/pci/cx88/cx88-blackbird.c  |  4 +---
 drivers/media/pci/cx88/cx88-dvb.c|  4 +---
 drivers/media/pci/cx88/cx88-mpeg.c   | 11 +++
 drivers/media/pci/cx88/cx88-vbi.c|  9 +
 drivers/media/pci/cx88/cx88-video.c  | 18 +-
 drivers/media/pci/cx88/cx88.h|  2 ++
 drivers/media/platform/vivid/vivid-vid-out.c |  2 +-
 drivers/media/v4l2-core/v4l2-ioctl.c |  6 ++
 8 files changed, 28 insertions(+), 28 deletions(-)
--
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/RFC v9 03/19] mfd: max77693: Modify flash cell name identifiers

2014-12-09 Thread Jacek Anaszewski

On 12/09/2014 09:52 AM, Lee Jones wrote:

On Wed, 03 Dec 2014, Jacek Anaszewski wrote:


Change flash cell identifiers from max77693-flash to max77693-led
to avoid confusion with NOR/NAND Flash.

Signed-off-by: Jacek Anaszewski 
Acked-by: Kyungmin Park 
Cc: Chanwoo Choi 
Cc: Lee Jones 
---
  drivers/mfd/max77693.c |4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/mfd/max77693.c b/drivers/mfd/max77693.c
index a159593..cb14afa 100644
--- a/drivers/mfd/max77693.c
+++ b/drivers/mfd/max77693.c
@@ -53,8 +53,8 @@ static const struct mfd_cell max77693_devs[] = {
.of_compatible = "maxim,max77693-haptic",
},
{
-   .name = "max77693-flash",
-   .of_compatible = "maxim,max77693-flash",
+   .name = "max77693-led",
+   .of_compatible = "maxim,max77693-led",


This is fine by me, so long as you've been through the usual
deprecation procedures or this platform is still WiP.


It was me who added of_compatible for max77693-flash, but the
related led driver has not been yet merged and there are no
other drivers depending on it.

Best Regards,
Jacek Anaszewski

--
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/RFC v9 04/19] mfd: max77693: adjust max77693_led_platform_data

2014-12-09 Thread Jacek Anaszewski

On 12/09/2014 09:50 AM, Lee Jones wrote:

On Wed, 03 Dec 2014, Jacek Anaszewski wrote:


Add "label" array for Device Tree strings with the name of a LED device
and make flash_timeout a two element array, for caching the sub-led
related flash timeout. Added is also an array for caching pointers to the
sub-nodes representing sub-leds.

Signed-off-by: Jacek Anaszewski 
Acked-by: Kyungmin Park 
Cc: Chanwoo Choi 
Cc: Lee Jones 
---
  include/linux/mfd/max77693.h |4 +++-
  1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/include/linux/mfd/max77693.h b/include/linux/mfd/max77693.h
index f0b6585..c80ee99 100644
--- a/include/linux/mfd/max77693.h
+++ b/include/linux/mfd/max77693.h
@@ -88,16 +88,18 @@ enum max77693_led_boost_mode {
  };

  struct max77693_led_platform_data {
+   const char *label[2];
u32 fleds[2];
u32 iout_torch[2];for_each_available_child_of_node
u32 iout_flash[2];
u32 trigger[2];
u32 trigger_type[2];
+   u32 flash_timeout[2];
u32 num_leds;
u32 boost_mode;
-   u32 flash_timeout;
u32 boost_vout;
u32 low_vsys;
+   struct device_node *sub_nodes[2];


I haven't seen anyone do this before.  Why can't you use the provided
OF functions to traverse through your tree?


I use for_each_available_child_of_node when parsing DT node, but I
need to cache the pointer to sub-node to be able to use it later
when it needs to be passed to V4L2 sub-device which is then
asynchronously matched by the phandle to sub-node.

If it is not well seen to cache it in the platform data then
I will find different way to accomplish this.

Best Regards,
Jacek Anaszewski
--
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/RFC v8 02/14] Documentation: leds: Add description of LED Flash class extension

2014-12-09 Thread Jacek Anaszewski

Hi Pavel,

On 12/08/2014 09:18 PM, Pavel Machek wrote:

On Mon 2014-12-08 17:55:20, Jacek Anaszewski wrote:

On 12/06/2014 01:43 PM, Pavel Machek wrote:



The format of a sysfs attribute should be concise.
The error codes are generic and map directly to the V4L2 Flash
error codes.



Actually I'd like to see those flash fault code defined in LED
subsystem. And V4L2 will just include LED flash header file to use it.
Because flash fault code is not for V4L2 specific but it's a feature
of LED flash devices.

For clearing error code of flash devices, I think it depends on the
hardware. If most of our LED flash is using reading to clear error
code, we probably can make it simple as this now. But what if some
other LED flash devices are using writing to clear error code? we
should provide a API to that?


Actually, we should provide API that makes sense, and that is easy to
use by userspace.

I believe "read" is called read because it does not change anything,
and it should stay that way in /sysfs. You may want to talk to sysfs
maintainers if you plan on doing another semantics.


How would you proceed in case of devices which clear their fault
register upon I2C readout (e.g. AS3645)? In this case read does have
a side effect. For such devices attribute semantics would have to be
different than for the devices which don't clear faults on readout.


No, semantics should be same for all devices.

If device clears fault register during I2C readout, kernel will simply
gather faults in an variable, and clear them upon write to sysfs file.


This approach would require implementing additional mechanisms on
both sides: LED Flash class core and a LED Flash class driver.
In the former the sysfs attribute write permissions would have
to be decided in the runtime and in the latter caching mechanism
would have to be implemented per driver. We would have to also
consider how to approach the issue in case of sub-leds.

The only reason for this overhead is trying to avoid side effects
on reading sysfs attribute. After weighing the pros and cons,
I am not sure if it is worthwhile.

Best Regards,
Jacek Anaszewski
--
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 3/9] clk: sunxi: Add prcm mod0 clock driver

2014-12-09 Thread Maxime Ripard
On Mon, Dec 08, 2014 at 09:19:02AM +0100, Hans de Goede wrote:
> Hi,
> 
> On 07-12-14 19:08, Maxime Ripard wrote:
> >On Wed, Dec 03, 2014 at 10:49:20AM +0100, Hans de Goede wrote:
> 
> 
> 
> >>So it should not have a simple-bus compatible either, and as such we cannot
> >>simply change the mod0 driver from of_clk_define to a platform driver 
> >>because
> >>then we need to instantiate platform devs for the mod0 clock nodes, which
> >>means making the clock node a simple-bus.
> >
> >I guess we can do that as a temporary measure until we get things
> >right on that front. I'm totally open to doing that work, so I'm not
> >asking you to do it.
> >
> >>I can see your logic in wanting the ir_clk prcm sub-node to use the
> >>mod0 compatible string, so how about we make the mod0 driver both
> >>register through of_declare and as a platform driver. Note this means
> >>that it will try to bind twice to the ir_clk node, since of_clk_declare
> >>will cause it to try and bind there too AFAIK.
> >
> >Hmmm, I could live with that for a while too. That shouldn't even
> >require too much work, since the first thing we check in the mod0 code
> >is that we actually have something in reg, which will not be the case
> >in the OF_CLK_DECLARE case.
> >
> >>The of_clk_declare bind will fail though because there is no regs
> >>property, so this double bind is not an issue as long as we do not
> >>log errors on the first bind failure.
> >
> >Yep, exactly.
> >
> >>Note that the ir_clk node will still need an "ir-clk" compatible as
> >>well for the MFD to find it and assign the proper resources to it.
> >
> >No, it really doesn't. At least for now, we have a single mod0 clock
> >under the PRCM MFD. If (and only if) one day, we find ourselves in a
> >position where we have two mod0 clocks under the PRCM, then we'll fix
> >the MFD code to deal with that, because it really should deal with it.
> 
> Ok, using only the mod0 compat string works for me. I'll respin my
> patch-set (minus the one patch you've already merged) to make the modo
> clk driver use both of_clk_declare and make it a platfrom driver, and
> use the mod0 compat string for the ir-clk node.
> 
> Not sure when I'll get this done exactly though, but we still have
> a while before 3.20 :)

Indeed :)

Thanks!
Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com


signature.asc
Description: Digital signature


Re: [PATCH/RFC v9 03/19] mfd: max77693: Modify flash cell name identifiers

2014-12-09 Thread Lee Jones
On Wed, 03 Dec 2014, Jacek Anaszewski wrote:

> Change flash cell identifiers from max77693-flash to max77693-led
> to avoid confusion with NOR/NAND Flash.
> 
> Signed-off-by: Jacek Anaszewski 
> Acked-by: Kyungmin Park 
> Cc: Chanwoo Choi 
> Cc: Lee Jones 
> ---
>  drivers/mfd/max77693.c |4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/mfd/max77693.c b/drivers/mfd/max77693.c
> index a159593..cb14afa 100644
> --- a/drivers/mfd/max77693.c
> +++ b/drivers/mfd/max77693.c
> @@ -53,8 +53,8 @@ static const struct mfd_cell max77693_devs[] = {
>   .of_compatible = "maxim,max77693-haptic",
>   },
>   {
> - .name = "max77693-flash",
> - .of_compatible = "maxim,max77693-flash",
> + .name = "max77693-led",
> + .of_compatible = "maxim,max77693-led",

This is fine by me, so long as you've been through the usual
deprecation procedures or this platform is still WiP.

>   },
>  };
>  

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
--
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/RFC v9 04/19] mfd: max77693: adjust max77693_led_platform_data

2014-12-09 Thread Lee Jones
On Wed, 03 Dec 2014, Jacek Anaszewski wrote:

> Add "label" array for Device Tree strings with the name of a LED device
> and make flash_timeout a two element array, for caching the sub-led
> related flash timeout. Added is also an array for caching pointers to the
> sub-nodes representing sub-leds.
> 
> Signed-off-by: Jacek Anaszewski 
> Acked-by: Kyungmin Park 
> Cc: Chanwoo Choi 
> Cc: Lee Jones 
> ---
>  include/linux/mfd/max77693.h |4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/include/linux/mfd/max77693.h b/include/linux/mfd/max77693.h
> index f0b6585..c80ee99 100644
> --- a/include/linux/mfd/max77693.h
> +++ b/include/linux/mfd/max77693.h
> @@ -88,16 +88,18 @@ enum max77693_led_boost_mode {
>  };
>  
>  struct max77693_led_platform_data {
> + const char *label[2];
>   u32 fleds[2];
>   u32 iout_torch[2];
>   u32 iout_flash[2];
>   u32 trigger[2];
>   u32 trigger_type[2];
> + u32 flash_timeout[2];
>   u32 num_leds;
>   u32 boost_mode;
> - u32 flash_timeout;
>   u32 boost_vout;
>   u32 low_vsys;
> + struct device_node *sub_nodes[2];

I haven't seen anyone do this before.  Why can't you use the provided
OF functions to traverse through your tree?

>  };
>  
>  /* MAX77693 */

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
--
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 3/5] media: ov2640: add primary dt support

2014-12-09 Thread Laurent Pinchart
Hi Josh,

On Tuesday 09 December 2014 11:10:01 Josh Wu wrote:
> On 12/9/2014 2:39 AM, Laurent Pinchart wrote:
> > Hi Josh,
> > 
> > Thank you for the patch.
> > 
> > On Monday 08 December 2014 19:29:05 Josh Wu wrote:
> >> Add device tree support for ov2640.
> >> 
> >> Cc: devicet...@vger.kernel.org
> >> Signed-off-by: Josh Wu 
> >> ---
> >> 
> >> v1 -> v2:
> >>1. use gpiod APIs.
> >>2. change the gpio pin's name according to datasheet.
> >>3. reduce the delay for .reset() function.
> >>   
> >>   drivers/media/i2c/soc_camera/ov2640.c | 86 +---
> >>   1 file changed, 80 insertions(+), 6 deletions(-)
> >> 
> >> diff --git a/drivers/media/i2c/soc_camera/ov2640.c
> >> b/drivers/media/i2c/soc_camera/ov2640.c index 9ee910d..2a57979 100644
> >> --- a/drivers/media/i2c/soc_camera/ov2640.c
> >> +++ b/drivers/media/i2c/soc_camera/ov2640.c

[snip]

> >> +static int ov2640_probe_dt(struct i2c_client *client,
> >> +  struct ov2640_priv *priv)
> >> +{
> >> +  priv->resetb_gpio = devm_gpiod_get_optional(&client->dev, "resetb",
> >> +  GPIOD_OUT_HIGH);
> >> +  if (!priv->resetb_gpio)
> >> +  dev_warn(&client->dev, "resetb gpio not found!\n");
> > 
> > No need to warn here, it's perfectly fine if the reset signal isn't
> > connected to a GPIO.
> 
> I want to print some information if no GPIO is assigned. So I'd like use
> dev_dbg() here.
> What do you feel?

If you want to print a message in that case dev_dbg is the most appropriate 
log level. I would skip it completely, but that's up to you.

> >> +  else if (IS_ERR(priv->resetb_gpio))
> >> +  return -EINVAL;
> >> +
> >> +  priv->pwdn_gpio = devm_gpiod_get_optional(&client->dev, "pwdn",
> >> +  GPIOD_OUT_HIGH);
> >> +  if (!priv->pwdn_gpio)
> >> +  dev_warn(&client->dev, "pwdn gpio not found!\n");
> > 
> > Same here.
> 
> ditto.
> 
> >> +  else if (IS_ERR(priv->pwdn_gpio))
> >> +  return -EINVAL;
> >> +
> >> +  /* Initialize the soc_camera_subdev_desc */
> >> +  priv->ssdd_dt.power = ov2640_hw_power;
> >> +  priv->ssdd_dt.reset = ov2640_hw_reset;
> >> +  client->dev.platform_data = &priv->ssdd_dt;
> >> +
> >> +  return 0;
> >> +}
> >> +
> >>   /*
> >>* i2c_driver functions
> >>*/

-- 
Regards,

Laurent Pinchart

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