cron job: media_tree daily build: WARNINGS
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 Nov 6 04:00:21 CET 2013 git branch: for-v3.13c git hash: 3adeac2c34cc28e05d0ec52f38f009dcce278555 gcc version:i686-linux-gcc (GCC) 4.8.1 sparse version: 0.4.5-rc1 host hardware: x86_64 host os:3.11-6.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.31.14-i686: 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-rc1-i686: OK linux-2.6.31.14-x86_64: 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-rc1-x86_64: OK apps: WARNINGS spec-git: OK sparse version: 0.4.5-rc1 sparse: 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: [PATCH 2/6] [media] mt9p031: Include linux/of.h header
Hi Sachin, Thank you for the patch, and sorry for the late reply. On Friday 18 October 2013 08:37:11 Sachin Kamat wrote: > 'of_match_ptr' is defined in linux/of.h. Include it explicitly to avoid > build breakage in the future. > > Signed-off-by: Sachin Kamat > Cc: Laurent Pinchart Acked-by: Laurent Pinchart I've taken the patch in my tree and will push it upstream. > --- > drivers/media/i2c/mt9p031.c |1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/media/i2c/mt9p031.c b/drivers/media/i2c/mt9p031.c > index 4734836..1c2303d 100644 > --- a/drivers/media/i2c/mt9p031.c > +++ b/drivers/media/i2c/mt9p031.c > @@ -19,6 +19,7 @@ > #include > #include > #include > +#include > #include > #include > #include -- 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
Re: [PATCH 07/19] v4l: sh_vou: Enable the driver on all ARM platforms
Hi Mauro, On Wednesday 30 October 2013 10:26:23 Mauro Carvalho Chehab wrote: > Em Tue, 29 Oct 2013 00:46:55 +0100 Laurent Pinchart escreveu: > > Renesas ARM platforms are transitioning from single-platform to > > multi-platform kernels using the new ARCH_SHMOBILE_MULTI. Make the > > driver available on all ARM platforms to enable it on both ARCH_SHMOBILE > > and ARCH_SHMOBILE_MULTI and increase build testing coverage. > > > > Cc: Mauro Carvalho Chehab > > Cc: linux-media@vger.kernel.org > > Signed-off-by: Laurent Pinchart > > > > I'm understanding that the plan is to commit it via an ARM tree, right? Actually the plan is to get this upstream through you tree :-) However, I'm trying a different approach to the problem, so I'll post a new version of the patch set in the near future. > If so: > Acked-by: Mauro Carvalho Chehab > > PS.: With regards to the discussions about this patch series, I'm ok on > having this enabled for all archs or just for the archs that are known have > this IP block, of course provided that not includes to march are there. > > The rationale is that, in the specific case of V4L, the platform drivers are > already on a separate Kconfig menu, with makes no sense to be enabled on any > non SoC configuration. We will likely split dependencies on two lines in Kconfig, one for the build- time dependencies and one for the runtime dependencies. A driver that compiles on ARM only and supports hardware that is present on ARCH_SHMOBILE SoCs only would thus have depends on ARM depends on ARCH_SHMOBILE || COMPILE_TEST Build-time dependencies on other software components (I2C for instance) would be listed on the first line. The code below would become depends on VIDEO_DEV && I2C depends on ARCH_SHMOBILE || COMPILE_TEST > > --- > > > > drivers/media/platform/Kconfig | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/media/platform/Kconfig > > b/drivers/media/platform/Kconfig index c7caf94..a726f86 100644 > > --- a/drivers/media/platform/Kconfig > > +++ b/drivers/media/platform/Kconfig > > @@ -36,7 +36,7 @@ source "drivers/media/platform/blackfin/Kconfig" > > config VIDEO_SH_VOU > > tristate "SuperH VOU video output driver" > > depends on MEDIA_CAMERA_SUPPORT > > - depends on VIDEO_DEV && ARCH_SHMOBILE && I2C > > + depends on VIDEO_DEV && ARM && I2C > > select VIDEOBUF_DMA_CONTIG > > help > > Support for the Video Output Unit (VOU) on SuperH SoCs. -- 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
Re: [early RFC] Device Tree bindings for OMAP3 Camera Subsystem
Hi Sebastian, Thank you for the aptch. On Sunday 03 November 2013 23:03:15 Sebastian Reichel wrote: > Hi, > > This is an early RFC for omap3isp DT support. For now i just created a > potential DT binding documentation based on the existing platform data: > > Binding for the OMAP3 Camera subsystem with the image signal processor (ISP) > feature. > > omap3isp node > - > > Required properties: > > - compatible : should be "ti,omap3isp" for OMAP3; > - reg : physical addresses and length of the registers set; > - clocks : list of clock specifiers, corresponding to entries in > clock-names property; > - clock-names : must contain "cam_ick", "cam_mclk", "csi2_96m_fck", > "l3_ick" entries, matching entries in the clocks property; According to the OMAP36xx TRM, the ISP functional and interface clocks are called CAM_FCLK and CAM_ICLK. They are driven by L3_ICLK and L4_ICLK respectively, and both gated through a single bit. The OMAP platform code instantiates a cam_ick clock for CAM_ICLK but doesn't create any clock for CAM_FCLK. The reason is probably that such a clock wasn't really needed, as enabling the interface clock enables the functional clock anyway. Now that we're moving to DT the clock names will be set in stone, so maybe we should think about them a bit. Would it make sense to rename the clocks according to the names used in the OMAP36xx TRM ? We should probably check the documentation of the other SoCs in which the ISP is used to verify whether the names match. Would it also make sense to create an FCLK clock and use it instead of l3_ick ? > - interrupts : must contain mmu interrupt; > - ti,iommu: phandle to isp mmu; Are there DT bindings for the IOMMU ? They don't seem to be present in mainline. > Optional properties: > > - VDD_CSIPHY1-supply : regulator for csi phy1 > - VDD_CSIPHY2-supply : regulator for csi phy2 Should the regulators be renamed to lower-case ? > - ti,isp-xclk-1 : device(s) attached to ISP's first external > clock > - ti,isp-xclk-2 : device(s) attached to ISP's second external > clock That information should be present in the clock client node, not the ISP node. > device-group subnode > > > Required properties: > - ti,isp-interface-type : Integer describing the interface type, one of > the > following * 0 = ISP_INTERFACE_PARALLEL >* 1 = ISP_INTERFACE_CSI2A_PHY2 >* 2 = ISP_INTERFACE_CCP2B_PHY1 >* 3 = ISP_INTERFACE_CCP2B_PHY2 >* 4 = ISP_INTERFACE_CSI2C_PHY1 > - ti,isp-devices : Array of phandles to devices connected via the > interface Is there any reason why you don't use the V4L2 DT bindings to describe the pipeline ? > - One of the following configuration nodes (depending on > ti,isp-interface-type) - ti,ccp2-bus-cfg : CCP2 bus configuration (needed > for ISP_INTERFACE_CCP*) - ti,parallel-bus-cfg : PARALLEL bus configuration > (needed for ISP_INTERFACE_PARALLEL) - ti,csi2-bus-cfg : CSI bus > configuration (needed for ISP_INTERFACE_CSI*) > > ccp2-bus-cfg subnode > > > Required properties: > - ti,video-port-clock-divisor : integer; used for video port output clock > control > > Optional properties: > - ti,inverted-clock : boolean; clock/strobe signal is inverted > - ti,enable-crc : boolean; enable crc checking > - ti,ccp2-mode-mipi : boolean; port is used in MIPI-CSI1 mode > (default: > CCP2 mode) - ti,phy-layer-is-strobe : boolean; use data/strobe physical > layer (default: data/clock physical layer) - ti,data-lane-configuration > : > integer array with position and polarity information for lane 1 and 2 - > ti,clock-lane-configuration : integer array with position and polarity > information for clock lane > > parallel-bus-cfg subnode > > > Required properties: > - ti,data-lane-shift : integer; shift data lanes by > this amount > > Optional properties: > - ti,clock-falling-edge : boolean; sample on > falling edge (default: > rising edge) - ti,horizontal-synchronization-active-low : boolean; > default: > active high - ti,vertical-synchronization-active-low : boolean; default: > active high - ti,data-polarity-ones-complement: boolean; data > polarity is > one's complement > > csi2-bus-cfg subnode > > > Required properties: > - ti,video-port-clock-divisor : integer; used for video port output clock > control > > Optional properties: > - ti,data-lane-configuration : integer array with position and polarity > information for lane 1 and 2 - ti,clock-lane-configuration: integer array > with position and polarity information for clock lane - ti,enable-crc > : > boolean; enable crc checking > > Example for Nokia N900 > -- > > omap3isp: isp@480BC000 { > com
Re: [PATCH 06/24] V4L2: add a common V4L2 subdevice platform data type
Hi Hans, On Monday 04 November 2013 12:24:10 Hans Verkuil wrote: > Hi Guennadi, > > Sorry for the delay, I only saw this today while I was going through my > mail backlog. > > On 10/17/2013 08:24 PM, Guennadi Liakhovetski wrote: > > Hi Hans > > > > Sorry for reviving this old thread. I was going to resubmit a part of > > those patches for mainlining and then I found this your comment, which I > > didn't reply to back then. > > > > On Fri, 19 Apr 2013, Hans Verkuil wrote: > >> On Fri April 19 2013 09:48:27 Guennadi Liakhovetski wrote: > >>> Hi Hans > >>> > >>> Thanks for reviewing. > >>> > >>> On Fri, 19 Apr 2013, Hans Verkuil wrote: > On Thu April 18 2013 23:35:27 Guennadi Liakhovetski wrote: > > This struct shall be used by subdevice drivers to pass per-subdevice > > data, e.g. power supplies, to generic V4L2 methods, at the same time > > allowing optional host-specific extensions via the host_priv pointer. > > To avoid having to pass two pointers to those methods, add a pointer > > to this new struct to struct v4l2_subdev. > > > > Signed-off-by: Guennadi Liakhovetski > > --- > > > > include/media/v4l2-subdev.h | 13 + > > 1 files changed, 13 insertions(+), 0 deletions(-) > > > > diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h > > index eb91366..b15c6e0 100644 > > --- a/include/media/v4l2-subdev.h > > +++ b/include/media/v4l2-subdev.h > > @@ -561,6 +561,17 @@ struct v4l2_subdev_internal_ops { > > > > /* Set this flag if this subdev generates events. */ > > #define V4L2_SUBDEV_FL_HAS_EVENTS (1U << 3) > > > > +struct regulator_bulk_data; > > + > > +struct v4l2_subdev_platform_data { > > + /* Optional regulators uset to power on/off the subdevice */ > > + struct regulator_bulk_data *regulators; > > + int num_regulators; > > + > > + /* Per-subdevice data, specific for a certain video host device > > */ > > + void *host_priv; > > +}; > > + > > > > /* Each instance of a subdev driver should create this struct, either > > stand-alone or embedded in a larger struct. > > */ > > @@ -589,6 +600,8 @@ struct v4l2_subdev { > > > > /* pointer to the physical device */ > > struct device *dev; > > struct v4l2_async_subdev_list asdl; > > > > + /* common part of subdevice platform data */ > > + struct v4l2_subdev_platform_data *pdata; > > > > }; > > > > static inline struct v4l2_subdev *v4l2_async_to_subdev( > > Sorry, this is the wrong approach. > > This is data that is of no use to the subdev driver itself. It really > is v4l2_subdev_host_platform_data, and as such must be maintained by > the bridge driver. > >>> > >>> I don't think so. It has been discussed and agreed upon, that only > >>> subdevice drivers know when to switch power on and off, because only > >>> they know when they need to access the hardware. So, they have to manage > >>> regulators. In fact, those regulators supply power to respective > >>> subdevices, e.g. a camera sensor. Why should the bridge driver manage > >>> them? The V4L2 core can (and probably should) provide helper functions > >>> for that, like soc-camera currently does, but in any case it's the > >>> subdevice driver, that has to call them. > >> > >> Ah, OK. I just realized I missed some context there. I didn't pay much > >> attention to the regulator discussions since that's not my area of > >> expertise. > >> > >> In that case my only comment is to drop the host_priv pointer since that > >> just duplicates v4l2_get/set_subdev_hostdata(). > > > > I think it's different. This is _platform_ data, whereas struct > > v4l2_subdev::host_priv is more like run-time data. > > You mean subdev_hostdata() instead of host_priv, right? > > > This field is for the > > per-subdevice host-specific data, that the platform has to pass to the > > host driver. In the soc-camera case this is the largest bulk of the data, > > that platforms currently pass to the soc-camera framework in the host part > > of struct soc_camera_link. This data most importantly includes I2C > > information. Yes, this _could_ be passed to soc-camera separately from the > > host driver, but that would involve quite some refactoring of the "legacy" > > synchronous probing mode, which I'd like to avoid if possible. This won't > > be used in the asynchronous case. Do you think we can keep this pointer in > > this sruct? We could rename it to avoid confusion with the field, that you > > told about. > > I'm wondering: do we need host_priv at all? Can't drivers use container_of > to go from struct v4l2_subdev_platform_data to the platform_data struct > containing v4l2_subdev_platform_data? > > That would be a cleaner solution IMHO. Using host_priv basically f
Re: ivtv 1.4.2/1.4.3 broken in recent kernels?
On Mon, 2013-11-04 at 13:44 +0100, Hans Verkuil wrote: > On 10/19/2013 07:09 PM, Andy Walls wrote: > > On Wed, 2013-10-16 at 01:10 +0100, Rajil Saraswat wrote: > > Try applying the following (untested) patch that is made against the > > bleeding edge Linux kernel. The test on the mute control state in > > wm8775_s_routing() appears to have been inverted in the bad commit you > > isolated. > > Aargh! I'm pretty sure that's the culprit. Man, that's been broken for ages. Hi Hans, Yes, and only *one* person reported it in those years. I suspect very few people use the comination of conventional PCI, analog video, and SVideo 2 or Composite 2 anymore. > I'll see if I can test this patch this week. Thanks! I'm very busy at work until mid-December. Regards, Andy -- 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 v3 04/29] [media] cx18: struct i2c_client is too big for stack
On Tue, 2013-11-05 at 08:01 -0200, Mauro Carvalho Chehab wrote: > drivers/media/pci/cx18/cx18-driver.c: In function 'cx18_read_eeprom': > drivers/media/pci/cx18/cx18-driver.c:357:1: warning: the frame size of > 1072 bytes is larger than 1024 bytes [-Wframe-larger-than=] > That happens because the routine allocates 256 bytes for an eeprom buffer, > plus > the size of struct i2c_client, with is big. > Change the logic to dynamically allocate/deallocate space for struct > i2c_client, > instead of using the stack. > > Signed-off-by: Mauro Carvalho Chehab > --- > drivers/media/pci/cx18/cx18-driver.c | 20 > 1 file changed, 12 insertions(+), 8 deletions(-) > > diff --git a/drivers/media/pci/cx18/cx18-driver.c > b/drivers/media/pci/cx18/cx18-driver.c > index ff7232023f56..87f5bcf29e90 100644 > --- a/drivers/media/pci/cx18/cx18-driver.c > +++ b/drivers/media/pci/cx18/cx18-driver.c > @@ -324,23 +324,24 @@ static void cx18_eeprom_dump(struct cx18 *cx, unsigned > char *eedata, int len) > /* Hauppauge card? get values from tveeprom */ > void cx18_read_eeprom(struct cx18 *cx, struct tveeprom *tv) > { > - struct i2c_client c; > + struct i2c_client *c; > u8 eedata[256]; > > - memset(&c, 0, sizeof(c)); > - strlcpy(c.name, "cx18 tveeprom tmp", sizeof(c.name)); > - c.adapter = &cx->i2c_adap[0]; > - c.addr = 0xA0 >> 1; > + c = kzalloc(sizeof(*c), GFP_ATOMIC); Hi Mauro, GFP_ATOMIC seems overly strict, as this function is not in called in an atomic context AFAIK. Maybe use GFP_TEMPORARY or GFP_KERNEL. Regards, Andy > + > + strlcpy(c->name, "cx18 tveeprom tmp", sizeof(c->name)); > + c->adapter = &cx->i2c_adap[0]; > + c->addr = 0xa0 >> 1; > > memset(tv, 0, sizeof(*tv)); > - if (tveeprom_read(&c, eedata, sizeof(eedata))) > - return; > + if (tveeprom_read(c, eedata, sizeof(eedata))) > + goto ret; > > switch (cx->card->type) { > case CX18_CARD_HVR_1600_ESMT: > case CX18_CARD_HVR_1600_SAMSUNG: > case CX18_CARD_HVR_1600_S5H1411: > - tveeprom_hauppauge_analog(&c, tv, eedata); > + tveeprom_hauppauge_analog(c, tv, eedata); > break; > case CX18_CARD_YUAN_MPC718: > case CX18_CARD_GOTVIEW_PCI_DVD3: > @@ -354,6 +355,9 @@ void cx18_read_eeprom(struct cx18 *cx, struct tveeprom > *tv) > cx18_eeprom_dump(cx, eedata, sizeof(eedata)); > break; > } > + > +ret: > + kfree(c); > } > > static void cx18_process_eeprom(struct cx18 *cx) -- 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] Full DVB driver package for Earthsoft PT3 (ISDB-S/T) cards
Michael Krufky linuxtv.org> writes: > As the DVB maintainer, I am telling you that I won't merge this as a > monolithic driver. The standard is to separate the driver into > modules where possible, unless there is a valid reason for doing > otherwise. > > I understand that you used the PT1 driver as a reference, but we're > trying to enforce a standard of codingstyle within the kernel. I > recommend looking at the other DVB drivers as well. OK Sir. Any good / latest examples? > Please shorten it to something more along the lines of: > > Support for Earthsoft PT3 PCI-Express cards. > > Say Y or M to enable support for this device. Roger > > FYI, there is another version of PT3 driver, named pt3_drv.ko, that > > utilize character devices as the I/O. I'd rather use pt3_dvb.ko to > > distinguish. > > we're not interested in multiple drivers for the same hardware. Only > one will be merged into the kernel, if any at all, and users need not > think about the names of these drivers. One of the beauties of > merging a driver into the kernel is that users gain automatic support > for the hardware without having to think or care about the name of the > driver. pt3_drv.ko is a public domain (old-fashioned) chardev driver for PT3, and does not conform to standard DVB platform. It doesn't seem to be merged into the mainstreem kernel tree. > > Maybe I'd like to change the dirname: > > drivers/media/pci/pt3_dvb => drivers/media/pci/pt3 > > not a bad idea > >> every source file and header file should include GPLv2 license headers. > > > > Roger: not very crucial though... > > entirely crucial if you're looking to merge into the kernel. ...or > did we misunderstand your request? I meant: not a big task... is the following enough? /* * DVB driver for Earthsoft PT3 PCI-E ISDB-S/T card * * Copyright (C) 2013 * * This program is free software; you can redistribute it and/or modify * it under the terms of the GNU General Public License as published by * the Free Software Foundation; either version 2 of the License, or * (at your option) any later version. * * This program is distributed in the hope that it will be useful, * but WITHOUT ANY WARRANTY; without even the implied warranty of * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the * GNU General Public License for more details. * * You should have received a copy of the GNU General Public License * along with this program; if not, write to the Free Software * Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA. */ > >>> +#define PT3_QM_INIT_DUMMY_RESET 0x0c > >> > >> it's nicer when these macros are defined in one place, but its not a > >> requirement. It's OK to leave it here if you really want to, but I > >> suggest instead to create a _reg.h file containing all register > >> #defines > > > > Will consider... -- 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 4/4] media/solo6x10: Changes on the vb2-dma-sg API
On Fri, 19 Jul 2013 09:58:49 +0200 Ricardo Ribalda Delgado wrote: > The struct vb2_dma_sg_desc has been replaced with the generic sg_table > to describe the location of the video buffers. > > Signed-off-by: Ricardo Ribalda Delgado Acked-by: Ismael Luceno <...> signature.asc Description: PGP signature
Re: [PATCH] SOLO6x10: don't do DMA from stack in solo_dma_vin_region().
On Thu, 12 Sep 2013 14:25:36 +0200 khal...@piap.pl (Krzysztof Hałasa) wrote: > Signed-off-by: Krzysztof Hałasa Acked-by: Ismael Luceno <...> signature.asc Description: PGP signature
Re: [PATCH] SOLO6x10: Fix video encoding on big-endian systems.
On Thu, 12 Sep 2013 14:28:07 +0200 khal...@piap.pl (Krzysztof Hałasa) wrote: > Signed-off-by: Krzysztof Hałasa > Acked-by: Ismael Luceno <...> signature.asc Description: PGP signature
Re: [PATCH] SOLO6x10: Remove unused #define SOLO_DEFAULT_GOP
On Thu, 12 Sep 2013 14:26:46 +0200 khal...@piap.pl (Krzysztof Hałasa) wrote: > Signed-off-by: Krzysztof Hałasa > Acked-by: Ismael Luceno <...> signature.asc Description: PGP signature
Re: [PATCH] SOLO6x10: Fix video frame type (I/P/B).
On Mon, 07 Oct 2013 13:33:55 +0200 khal...@piap.pl (Krzysztof Hałasa) wrote: > Signed-off-by: Krzysztof Hałasa Acked-by: Ismael Luceno <...> signature.asc Description: PGP signature
Re: [PATCH] SOLO6x10: Fix video headers on certain hardware.
On Thu, 12 Sep 2013 14:43:34 +0200 khal...@piap.pl (Krzysztof Hałasa) wrote: > On certain platforms a sequence of dma_map_sg() and dma_unmap_sg() > discards data previously stored in the buffers. Build video headers > only after the DMA is completed. > > Signed-off-by: Krzysztof Hałasa > Acked-by: Ismael Luceno <...> signature.asc Description: PGP signature
Re: [PATCH] Full DVB driver package for Earthsoft PT3 (ISDB-S/T) cards
On Tue, Nov 5, 2013 at 3:56 PM, ほち wrote: > see inline > > 2013/11/6 Michael Krufky : >> responding inline: >> >>> Mauro Carvalho Chehab asked to put tuner code as an I2C driver, under >>> drivers/media/tuners, frontends at drivers/media/dvb- >>> However, to keep package integrity & compatibility with PT1/PT2 user apps, >>> FE etc. are still placed in the same directory. >> >> A userspace application doesn't care where file are places within the >> kernel tree. You are to use standard linux-dvb api's and the >> codingstyle of the driver shall comply with that of the linux kernel's >> DVB subsystem, including the proper placement of files. > > As stated in my (previous) mails, I took PT1 module as a reference. > Everything is located in single directory ...pt1/, including the frontends. > They are built as a single integrated module, earth-pt1.ko > > Sure I can split the FEs out to .../dvb-frontends/, build there as a separated > (FE only) module that would be hot-linked with the main module. However > I'm afraid (& sure) this will only make people confused with complex > dependencies leading to annoying bugs... The simpler the better... > > Guys I need more opinions from other people before splitting the module. > IMHO even Linus won't like this... > As the DVB maintainer, I am telling you that I won't merge this as a monolithic driver. The standard is to separate the driver into modules where possible, unless there is a valid reason for doing otherwise. I understand that you used the PT1 driver as a reference, but we're trying to enforce a standard of codingstyle within the kernel. I recommend looking at the other DVB drivers as well. >>> diff --git a/drivers/media/pci/pt3_dvb/Kconfig >>> b/drivers/media/pci/pt3_dvb/Kconfig >>> new file mode 100644 >>> index 000..f9ba00d >>> --- /dev/null >>> +++ b/drivers/media/pci/pt3_dvb/Kconfig >>> @@ -0,0 +1,12 @@ >>> +config PT3_DVB >>> + tristate "Earthsoft PT3 cards" >>> + depends on DVB_CORE && PCI >>> + help >>> + Support for Earthsoft PT3 PCI-Express cards. >>> + >>> + Since these cards have no MPEG decoder onboard, they transmit >>> + only compressed MPEG data over the PCI bus, so you need >>> + an external software decoder to watch TV on your computer. >>> + >>> + Say Y or M if you own such a device and want to use it. >> >> Very few of these digital tuner boards have onboard mpeg decoders. We >> can do without this extra information here. > > ok, will change to: > These cards transmit only compressed MPEG data over the PCI bus. > You need external software decoder to watch TV on your computer. This is superfluous information. Please look at the Kconfig description for the many other DVB drivers supported in the kernel. There are very few that contain their own hardware decoders, and almosy all of them require a software decoder for playback on a PC. Please shorten it to something more along the lines of: Support for Earthsoft PT3 PCI-Express cards. Say Y or M to enable support for this device. > >>> diff --git a/drivers/media/pci/pt3_dvb/Makefile >>> b/drivers/media/pci/pt3_dvb/Makefile >>> new file mode 100644 >>> index 000..7087c90 >>> --- /dev/null >>> +++ b/drivers/media/pci/pt3_dvb/Makefile >>> @@ -0,0 +1,6 @@ >>> +pt3_dvb-objs := pt3.o pt3_fe.o pt3_dma.o pt3_tc.o pt3_i2c.o pt3_bus.o >>> + >>> +obj-$(CONFIG_PT3_DVB) += pt3_dvb.o >>> + >>> +ccflags-y += -Idrivers/media/dvb-core -Idrivers/media/dvb-frontends >>> -Idrivers/media/tuners >>> + >> >> Ususally, the driver would be named 'pt3.ko' and the object file that >> handles the DVB api would be called pt3_dvb.o >> >> This isn't any rule, but the way that you've named them seems a bit >> awkward to me. I don't require that you change this, just stating my >> awkward feeling on the matter. > > FYI, there is another version of PT3 driver, named pt3_drv.ko, that > utilize character devices as the I/O. I'd rather use pt3_dvb.ko to > distinguish. > we're not interested in multiple drivers for the same hardware. Only one will be merged into the kernel, if any at all, and users need not think about the names of these drivers. One of the beauties of merging a driver into the kernel is that users gain automatic support for the hardware without having to think or care about the name of the driver. > Maybe I'd like to change the dirname: > drivers/media/pci/pt3_dvb => drivers/media/pci/pt3 not a bad idea ;-) > >>> +static int lnb = 2;/* used if not set by frontend / the value is >>> invalid */ >>> +module_param(lnb, int, 0); >>> +MODULE_PARM_DESC(lnb, "LNB level (0:OFF 1:+11V 2:+15V)"); >> >> Take these above three statements out of the header file and move them >> into a .c file > > OK Sir > >>> +struct dvb_frontend *pt3_fe_s_attach(struct pt3_adapter *adap); >>> +struct dvb_frontend *pt3_fe_t_attach(struct pt3_adapter *adap); >> >> Please create separate headers corresponding to the .c file that >> contains the functi
Re: [PATCH] Full DVB driver package for Earthsoft PT3 (ISDB-S/T) cards
see inline 2013/11/6 Michael Krufky : > responding inline: > >> Mauro Carvalho Chehab asked to put tuner code as an I2C driver, under >> drivers/media/tuners, frontends at drivers/media/dvb- >> However, to keep package integrity & compatibility with PT1/PT2 user apps, >> FE etc. are still placed in the same directory. > > A userspace application doesn't care where file are places within the > kernel tree. You are to use standard linux-dvb api's and the > codingstyle of the driver shall comply with that of the linux kernel's > DVB subsystem, including the proper placement of files. As stated in my (previous) mails, I took PT1 module as a reference. Everything is located in single directory ...pt1/, including the frontends. They are built as a single integrated module, earth-pt1.ko Sure I can split the FEs out to .../dvb-frontends/, build there as a separated (FE only) module that would be hot-linked with the main module. However I'm afraid (& sure) this will only make people confused with complex dependencies leading to annoying bugs... The simpler the better... Guys I need more opinions from other people before splitting the module. IMHO even Linus won't like this... >> diff --git a/drivers/media/pci/pt3_dvb/Kconfig >> b/drivers/media/pci/pt3_dvb/Kconfig >> new file mode 100644 >> index 000..f9ba00d >> --- /dev/null >> +++ b/drivers/media/pci/pt3_dvb/Kconfig >> @@ -0,0 +1,12 @@ >> +config PT3_DVB >> + tristate "Earthsoft PT3 cards" >> + depends on DVB_CORE && PCI >> + help >> + Support for Earthsoft PT3 PCI-Express cards. >> + >> + Since these cards have no MPEG decoder onboard, they transmit >> + only compressed MPEG data over the PCI bus, so you need >> + an external software decoder to watch TV on your computer. >> + >> + Say Y or M if you own such a device and want to use it. > > Very few of these digital tuner boards have onboard mpeg decoders. We > can do without this extra information here. ok, will change to: These cards transmit only compressed MPEG data over the PCI bus. You need external software decoder to watch TV on your computer. >> diff --git a/drivers/media/pci/pt3_dvb/Makefile >> b/drivers/media/pci/pt3_dvb/Makefile >> new file mode 100644 >> index 000..7087c90 >> --- /dev/null >> +++ b/drivers/media/pci/pt3_dvb/Makefile >> @@ -0,0 +1,6 @@ >> +pt3_dvb-objs := pt3.o pt3_fe.o pt3_dma.o pt3_tc.o pt3_i2c.o pt3_bus.o >> + >> +obj-$(CONFIG_PT3_DVB) += pt3_dvb.o >> + >> +ccflags-y += -Idrivers/media/dvb-core -Idrivers/media/dvb-frontends >> -Idrivers/media/tuners >> + > > Ususally, the driver would be named 'pt3.ko' and the object file that > handles the DVB api would be called pt3_dvb.o > > This isn't any rule, but the way that you've named them seems a bit > awkward to me. I don't require that you change this, just stating my > awkward feeling on the matter. FYI, there is another version of PT3 driver, named pt3_drv.ko, that utilize character devices as the I/O. I'd rather use pt3_dvb.ko to distinguish. Maybe I'd like to change the dirname: drivers/media/pci/pt3_dvb => drivers/media/pci/pt3 >> +static int lnb = 2;/* used if not set by frontend / the value is >> invalid */ >> +module_param(lnb, int, 0); >> +MODULE_PARM_DESC(lnb, "LNB level (0:OFF 1:+11V 2:+15V)"); > > Take these above three statements out of the header file and move them > into a .c file OK Sir >> +struct dvb_frontend *pt3_fe_s_attach(struct pt3_adapter *adap); >> +struct dvb_frontend *pt3_fe_t_attach(struct pt3_adapter *adap); > > Please create separate headers corresponding to the .c file that > contains the function. Don't put them all in one, as the tuner and > demodulator should be separate modules Splitting the protos? Well I will consider... > every source file and header file should include GPLv2 license headers. Roger: not very crucial though... >> +#define PT3_QM_INIT_DUMMY_RESET 0x0c > > it's nicer when these macros are defined in one place, but its not a > requirement. It's OK to leave it here if you really want to, but I > suggest instead to create a _reg.h file containing all register > #defines Will consider... -- 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: staging: media: Use dev_err() instead of pr_err()
Hello Dulshani Gunawardhana, The patch 44ee8e801137: "staging: media: Use dev_err() instead of pr_err()" from Oct 20, 2013, leads to the following GCC warning: drivers/staging/media/go7007/go7007-usb.c: In function ‘go7007_usb_probe’: drivers/staging/media/go7007/go7007-usb.c:1100:13: warning: ‘go’ may be used uninitialized in this function [-Wuninitialized] drivers/staging/media/go7007/go7007-usb.c 1049 static int go7007_usb_probe(struct usb_interface *intf, 1050 const struct usb_device_id *id) 1051 { 1052 struct go7007 *go; 1053 struct go7007_usb *usb; 1054 const struct go7007_usb_board *board; 1055 struct usb_device *usbdev = interface_to_usbdev(intf); 1056 unsigned num_i2c_devs; 1057 char *name; 1058 int video_pipe, i, v_urb_len; 1059 1060 dev_dbg(go->dev, "probing new GO7007 USB board\n"); ^^^ 1061 1062 switch (id->driver_info) { 1063 case GO7007_BOARDID_MATRIX_II: 1064 name = "WIS Matrix II or compatible"; 1065 board = &board_matrix_ii; 1066 break; There are several other uses of "go" before it has been initialized. Probably you will just want to change these back to pr_info(). Some of the messages are not very useful like: dev_info(go->dev, "Sensoray 2250 found\n"); You can delete that one. regards, dan carpenter -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC] [PATCH] media: marvell-ccic: use devm to release clk
On Tue, Nov 05, 2013 at 05:33:16PM +0800, lbyang wrote: > From: Libin Yang > Date: Tue, 5 Nov 2013 16:29:07 +0800 > Subject: [PATCH] media: marvell-ccic: use devm to release clk > > This patch uses devm to release the clks instead of releasing > manually. > And it adds enable/disable mipi_clk when getting its rate. > > Signed-off-by: Libin Yang The driver still is no beauty, but with this patch the clk usage at least seems to be API conformant. Acked-by: Uwe Kleine-König > --- > drivers/media/platform/marvell-ccic/mmp-driver.c | 39 > +- > 1 file changed, 8 insertions(+), 31 deletions(-) > > diff --git a/drivers/media/platform/marvell-ccic/mmp-driver.c > b/drivers/media/platform/marvell-ccic/mmp-driver.c > index 70cb57f..054507f 100644 > --- a/drivers/media/platform/marvell-ccic/mmp-driver.c > +++ b/drivers/media/platform/marvell-ccic/mmp-driver.c > @@ -142,12 +142,6 @@ static int mmpcam_power_up(struct mcam_camera > *mcam) > struct mmp_camera *cam = mcam_to_cam(mcam); > struct mmp_camera_platform_data *pdata; > > - if (mcam->bus_type == V4L2_MBUS_CSI2) { > - cam->mipi_clk = devm_clk_get(mcam->dev, "mipi"); > - if ((IS_ERR(cam->mipi_clk) && mcam->dphy[2] == 0)) > - return PTR_ERR(cam->mipi_clk); > - } > - > /* > * Turn on power and clocks to the controller. > */ > @@ -186,12 +180,6 @@ static void mmpcam_power_down(struct mcam_camera > *mcam) > gpio_set_value(pdata->sensor_power_gpio, 0); > gpio_set_value(pdata->sensor_reset_gpio, 0); > > - if (mcam->bus_type == V4L2_MBUS_CSI2 && !IS_ERR(cam->mipi_clk)) { > - if (cam->mipi_clk) > - devm_clk_put(mcam->dev, cam->mipi_clk); > - cam->mipi_clk = NULL; > - } > - > mcam_clk_disable(mcam); > } > > @@ -292,8 +280,9 @@ void mmpcam_calc_dphy(struct mcam_camera *mcam) > return; > > /* get the escape clk, this is hard coded */ > + clk_prepare_enable(cam->mipi_clk); > tx_clk_esc = (clk_get_rate(cam->mipi_clk) / 100) / 12; > - > + clk_disable_unprepare(cam->mipi_clk); > /* >* dphy[2] - CSI2_DPHY6: >* bit 0 ~ bit 7: CK Term Enable > @@ -325,19 +314,6 @@ static irqreturn_t mmpcam_irq(int irq, void *data) > return IRQ_RETVAL(handled); > } > > -static void mcam_deinit_clk(struct mcam_camera *mcam) > -{ > - unsigned int i; > - > - for (i = 0; i < NR_MCAM_CLK; i++) { > - if (!IS_ERR(mcam->clk[i])) { > - if (mcam->clk[i]) > - devm_clk_put(mcam->dev, mcam->clk[i]); > - } > - mcam->clk[i] = NULL; > - } > -} > - > static void mcam_init_clk(struct mcam_camera *mcam) > { > unsigned int i; > @@ -371,7 +347,6 @@ static int mmpcam_probe(struct platform_device > *pdev) > if (cam == NULL) > return -ENOMEM; > cam->pdev = pdev; > - cam->mipi_clk = NULL; > INIT_LIST_HEAD(&cam->devlist); > > mcam = &cam->mcam; > @@ -387,6 +362,11 @@ static int mmpcam_probe(struct platform_device > *pdev) > mcam->mclk_div = pdata->mclk_div; > mcam->bus_type = pdata->bus_type; > mcam->dphy = pdata->dphy; > + if (mcam->bus_type == V4L2_MBUS_CSI2) { > + cam->mipi_clk = devm_clk_get(mcam->dev, "mipi"); > + if ((IS_ERR(cam->mipi_clk) && mcam->dphy[2] == 0)) > + return PTR_ERR(cam->mipi_clk); > + } > mcam->mipi_enabled = false; > mcam->lane = pdata->lane; > mcam->chip_id = MCAM_ARMADA610; > @@ -444,7 +424,7 @@ static int mmpcam_probe(struct platform_device > *pdev) >*/ > ret = mmpcam_power_up(mcam); > if (ret) > - goto out_deinit_clk; > + return ret; > ret = mccic_register(mcam); > if (ret) > goto out_power_down; > @@ -469,8 +449,6 @@ out_unregister: > mccic_shutdown(mcam); > out_power_down: > mmpcam_power_down(mcam); > -out_deinit_clk: > - mcam_deinit_clk(mcam); > return ret; > } > > @@ -482,7 +460,6 @@ static int mmpcam_remove(struct mmp_camera *cam) > mmpcam_remove_device(cam); > mccic_shutdown(mcam); > mmpcam_power_down(mcam); > - mcam_deinit_clk(mcam); > return 0; > } > > -- > 1.7.9.5 > > > > -- Pengutronix e.K. | Uwe Kleine-König| Industrial Linux Solutions | http://www.pengutronix.de/ | -- 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 v3 25/29] [media] af9015: Don't use dynamic static allocation
Acked-by: Antti Palosaari Reviewed-by: Antti Palosaari Antti On 05.11.2013 12:01, Mauro Carvalho Chehab wrote: Dynamic static allocation is evil, as Kernel stack is too low, and compilation complains about it on some archs: drivers/media/usb/dvb-usb-v2/af9015.c:433:1: warning: 'af9015_eeprom_hash' uses dynamic stack allocation [enabled by default] In this specific case, it is a gcc bug, as the size is a const, but it is easy to just change it from const to a #define, getting rid of the gcc warning. Signed-off-by: Mauro Carvalho Chehab --- drivers/media/usb/dvb-usb-v2/af9015.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/media/usb/dvb-usb-v2/af9015.c b/drivers/media/usb/dvb-usb-v2/af9015.c index d556042cf312..da47d2392f2a 100644 --- a/drivers/media/usb/dvb-usb-v2/af9015.c +++ b/drivers/media/usb/dvb-usb-v2/af9015.c @@ -397,12 +397,13 @@ error: return ret; } +#define AF9015_EEPROM_SIZE 256 + /* hash (and dump) eeprom */ static int af9015_eeprom_hash(struct dvb_usb_device *d) { struct af9015_state *state = d_to_priv(d); int ret, i; - static const unsigned int AF9015_EEPROM_SIZE = 256; u8 buf[AF9015_EEPROM_SIZE]; struct req_t req = {READ_I2C, AF9015_I2C_EEPROM, 0, 0, 1, 1, NULL}; -- http://palosaari.fi/ -- 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 v3 26/29] [media] af9035: Don't use dynamic static allocation
Acked-by: Antti Palosaari Reviewed-by: Antti Palosaari Antti On 05.11.2013 12:01, Mauro Carvalho Chehab wrote: Dynamic static allocation is evil, as Kernel stack is too low, and compilation complains about it on some archs: drivers/media/usb/dvb-usb-v2/af9035.c:142:1: warning: 'af9035_wr_regs' uses dynamic stack allocation [enabled by default] drivers/media/usb/dvb-usb-v2/af9035.c:305:1: warning: 'af9035_i2c_master_xfer' uses dynamic stack allocation [enabled by default] Instead, let's enforce a limit for the buffer to be the max size of a control URB payload data (64 bytes). Signed-off-by: Mauro Carvalho Chehab --- drivers/media/usb/dvb-usb-v2/af9035.c | 29 ++--- 1 file changed, 26 insertions(+), 3 deletions(-) diff --git a/drivers/media/usb/dvb-usb-v2/af9035.c b/drivers/media/usb/dvb-usb-v2/af9035.c index 1ea17dc2a76e..c8fcd78425bd 100644 --- a/drivers/media/usb/dvb-usb-v2/af9035.c +++ b/drivers/media/usb/dvb-usb-v2/af9035.c @@ -21,6 +21,9 @@ #include "af9035.h" +/* Max transfer size done by I2C transfer functions */ +#define MAX_XFER_SIZE 64 + DVB_DEFINE_MOD_OPT_ADAPTER_NR(adapter_nr); static u16 af9035_checksum(const u8 *buf, size_t len) @@ -126,10 +129,16 @@ exit: /* write multiple registers */ static int af9035_wr_regs(struct dvb_usb_device *d, u32 reg, u8 *val, int len) { - u8 wbuf[6 + len]; + u8 wbuf[MAX_XFER_SIZE]; u8 mbox = (reg >> 16) & 0xff; struct usb_req req = { CMD_MEM_WR, mbox, sizeof(wbuf), wbuf, 0, NULL }; + if (6 + len > sizeof(wbuf)) { + dev_warn(&d->udev->dev, "%s: i2c wr: len=%d is too big!\n", +KBUILD_MODNAME, len); + return -EOPNOTSUPP; + } + wbuf[0] = len; wbuf[1] = 2; wbuf[2] = 0; @@ -228,9 +237,16 @@ static int af9035_i2c_master_xfer(struct i2c_adapter *adap, msg[1].len); } else { /* I2C */ - u8 buf[5 + msg[0].len]; + u8 buf[MAX_XFER_SIZE]; struct usb_req req = { CMD_I2C_RD, 0, sizeof(buf), buf, msg[1].len, msg[1].buf }; + + if (5 + msg[0].len > sizeof(buf)) { + dev_warn(&d->udev->dev, +"%s: i2c xfer: len=%d is too big!\n", +KBUILD_MODNAME, msg[0].len); + return -EOPNOTSUPP; + } req.mbox |= ((msg[0].addr & 0x80) >> 3); buf[0] = msg[1].len; buf[1] = msg[0].addr << 1; @@ -257,9 +273,16 @@ static int af9035_i2c_master_xfer(struct i2c_adapter *adap, msg[0].len - 3); } else { /* I2C */ - u8 buf[5 + msg[0].len]; + u8 buf[MAX_XFER_SIZE]; struct usb_req req = { CMD_I2C_WR, 0, sizeof(buf), buf, 0, NULL }; + + if (5 + msg[0].len > sizeof(buf)) { + dev_warn(&d->udev->dev, +"%s: i2c xfer: len=%d is too big!\n", +KBUILD_MODNAME, msg[0].len); + return -EOPNOTSUPP; + } req.mbox |= ((msg[0].addr & 0x80) >> 3); buf[0] = msg[0].len; buf[1] = msg[0].addr << 1; -- http://palosaari.fi/ -- 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 v3 18/29] [media] tuners: Don't use dynamic static allocation
Acked-by: Antti Palosaari Reviewed-by: Antti Palosaari Antti On 05.11.2013 12:01, Mauro Carvalho Chehab wrote: Dynamic static allocation is evil, as Kernel stack is too low, and compilation complains about it on some archs: drivers/media/tuners/e4000.c:50:1: warning: 'e4000_wr_regs' uses dynamic stack allocation [enabled by default] drivers/media/tuners/e4000.c:83:1: warning: 'e4000_rd_regs' uses dynamic stack allocation [enabled by default] drivers/media/tuners/fc2580.c:66:1: warning: 'fc2580_wr_regs.constprop.1' uses dynamic stack allocation [enabled by default] drivers/media/tuners/fc2580.c:98:1: warning: 'fc2580_rd_regs.constprop.0' uses dynamic stack allocation [enabled by default] drivers/media/tuners/tda18212.c:57:1: warning: 'tda18212_wr_regs' uses dynamic stack allocation [enabled by default] drivers/media/tuners/tda18212.c:90:1: warning: 'tda18212_rd_regs.constprop.0' uses dynamic stack allocation [enabled by default] drivers/media/tuners/tda18218.c:60:1: warning: 'tda18218_wr_regs' uses dynamic stack allocation [enabled by default] drivers/media/tuners/tda18218.c:92:1: warning: 'tda18218_rd_regs.constprop.0' uses dynamic stack allocation [enabled by default] Instead, let's enforce a limit for the buffer. Considering that I2C transfers are generally limited, and that devices used on USB has a max data length of 64 bytes for the control URBs. So, it seem safe to use 64 bytes as the hard limit for all those devices. On most cases, the limit is a way lower than that, butthis limit is small enough to not affect the Kernel stack, and it is a no brain limit, as using smaller ones would require to either carefully each driver or to take a look on each datasheet. Signed-off-by: Mauro Carvalho Chehab --- drivers/media/tuners/e4000.c| 21 +++-- drivers/media/tuners/fc2580.c | 21 +++-- drivers/media/tuners/tda18212.c | 21 +++-- drivers/media/tuners/tda18218.c | 21 +++-- 4 files changed, 76 insertions(+), 8 deletions(-) diff --git a/drivers/media/tuners/e4000.c b/drivers/media/tuners/e4000.c index ad9309da4a91..30192463c9e1 100644 --- a/drivers/media/tuners/e4000.c +++ b/drivers/media/tuners/e4000.c @@ -20,11 +20,14 @@ #include "e4000_priv.h" +/* Max transfer size done by I2C transfer functions */ +#define MAX_XFER_SIZE 64 + /* write multiple registers */ static int e4000_wr_regs(struct e4000_priv *priv, u8 reg, u8 *val, int len) { int ret; - u8 buf[1 + len]; + u8 buf[MAX_XFER_SIZE]; struct i2c_msg msg[1] = { { .addr = priv->cfg->i2c_addr, @@ -34,6 +37,13 @@ static int e4000_wr_regs(struct e4000_priv *priv, u8 reg, u8 *val, int len) } }; + if (1 + len > sizeof(buf)) { + dev_warn(&priv->i2c->dev, +"%s: i2c wr reg=%04x: len=%d is too big!\n", +KBUILD_MODNAME, reg, len); + return -EINVAL; + } + buf[0] = reg; memcpy(&buf[1], val, len); @@ -53,7 +63,7 @@ static int e4000_wr_regs(struct e4000_priv *priv, u8 reg, u8 *val, int len) static int e4000_rd_regs(struct e4000_priv *priv, u8 reg, u8 *val, int len) { int ret; - u8 buf[len]; + u8 buf[MAX_XFER_SIZE]; struct i2c_msg msg[2] = { { .addr = priv->cfg->i2c_addr, @@ -68,6 +78,13 @@ static int e4000_rd_regs(struct e4000_priv *priv, u8 reg, u8 *val, int len) } }; + if (len > sizeof(buf)) { + dev_warn(&priv->i2c->dev, +"%s: i2c rd reg=%04x: len=%d is too big!\n", +KBUILD_MODNAME, reg, len); + return -EINVAL; + } + ret = i2c_transfer(priv->i2c, msg, 2); if (ret == 2) { memcpy(val, buf, len); diff --git a/drivers/media/tuners/fc2580.c b/drivers/media/tuners/fc2580.c index 81f38aae9c66..430fa5163ec7 100644 --- a/drivers/media/tuners/fc2580.c +++ b/drivers/media/tuners/fc2580.c @@ -20,6 +20,9 @@ #include "fc2580_priv.h" +/* Max transfer size done by I2C transfer functions */ +#define MAX_XFER_SIZE 64 + /* * TODO: * I2C write and read works only for one single register. Multiple registers @@ -41,7 +44,7 @@ static int fc2580_wr_regs(struct fc2580_priv *priv, u8 reg, u8 *val, int len) { int ret; - u8 buf[1 + len]; + u8 buf[MAX_XFER_SIZE]; struct i2c_msg msg[1] = { { .addr = priv->cfg->i2c_addr, @@ -51,6 +54,13 @@ static int fc2580_wr_regs(struct fc2580_priv *priv, u8 reg, u8 *val, int len) } }; + if (1 + len > sizeof(buf)) { + dev_warn(&priv->i2c->dev, +"%s: i2c wr reg=%04x: len=%d is too big!\n", +KBUILD_MOD
Re: [PATCH v3 13/29] [media] dvb-frontends: Don't use dynamic static allocation
Acked-by: Antti Palosaari Reviewed-by: Antti Palosaari Antti On 05.11.2013 12:01, Mauro Carvalho Chehab wrote: Dynamic static allocation is evil, as Kernel stack is too low, and compilation complains about it on some archs: drivers/media/dvb-frontends/af9013.c:77:1: warning: 'af9013_wr_regs_i2c' uses dynamic stack allocation [enabled by default] drivers/media/dvb-frontends/af9033.c:188:1: warning: 'af9033_wr_reg_val_tab' uses dynamic stack allocation [enabled by default] drivers/media/dvb-frontends/af9033.c:68:1: warning: 'af9033_wr_regs' uses dynamic stack allocation [enabled by default] drivers/media/dvb-frontends/bcm3510.c:230:1: warning: 'bcm3510_do_hab_cmd' uses dynamic stack allocation [enabled by default] drivers/media/dvb-frontends/cxd2820r_core.c:84:1: warning: 'cxd2820r_rd_regs_i2c.isra.1' uses dynamic stack allocation [enabled by default] drivers/media/dvb-frontends/rtl2830.c:56:1: warning: 'rtl2830_wr' uses dynamic stack allocation [enabled by default] drivers/media/dvb-frontends/rtl2832.c:187:1: warning: 'rtl2832_wr' uses dynamic stack allocation [enabled by default] drivers/media/dvb-frontends/tda10071.c:52:1: warning: 'tda10071_wr_regs' uses dynamic stack allocation [enabled by default] drivers/media/dvb-frontends/tda10071.c:84:1: warning: 'tda10071_rd_regs' uses dynamic stack allocation [enabled by default] Instead, let's enforce a limit for the buffer. Considering that I2C transfers are generally limited, and that devices used on USB has a max data length of 64 bytes for the control URBs. So, it seem safe to use 64 bytes as the hard limit for all those devices. On most cases, the limit is a way lower than that, butthis limit is small enough to not affect the Kernel stack, and it is a no brain limit, as using smaller ones would require to either carefully each driver or to take a look on each datasheet. Signed-off-by: Mauro Carvalho Chehab --- drivers/media/dvb-frontends/af9013.c| 12 +++- drivers/media/dvb-frontends/af9033.c| 21 +++-- drivers/media/dvb-frontends/cxd2820r_core.c | 21 +++-- drivers/media/dvb-frontends/rtl2830.c | 12 +++- drivers/media/dvb-frontends/rtl2832.c | 12 +++- drivers/media/dvb-frontends/tda10071.c | 21 +++-- 6 files changed, 90 insertions(+), 9 deletions(-) diff --git a/drivers/media/dvb-frontends/af9013.c b/drivers/media/dvb-frontends/af9013.c index a204f2828820..19ba66ad23fa 100644 --- a/drivers/media/dvb-frontends/af9013.c +++ b/drivers/media/dvb-frontends/af9013.c @@ -24,6 +24,9 @@ #include "af9013_priv.h" +/* Max transfer size done by I2C transfer functions */ +#define MAX_XFER_SIZE 64 + struct af9013_state { struct i2c_adapter *i2c; struct dvb_frontend fe; @@ -50,7 +53,7 @@ static int af9013_wr_regs_i2c(struct af9013_state *priv, u8 mbox, u16 reg, const u8 *val, int len) { int ret; - u8 buf[3+len]; + u8 buf[MAX_XFER_SIZE]; struct i2c_msg msg[1] = { { .addr = priv->config.i2c_addr, @@ -60,6 +63,13 @@ static int af9013_wr_regs_i2c(struct af9013_state *priv, u8 mbox, u16 reg, } }; + if (3 + len > sizeof(buf)) { + dev_warn(&priv->i2c->dev, +"%s: i2c wr reg=%04x: len=%d is too big!\n", +KBUILD_MODNAME, reg, len); + return -EINVAL; + } + buf[0] = (reg >> 8) & 0xff; buf[1] = (reg >> 0) & 0xff; buf[2] = mbox; diff --git a/drivers/media/dvb-frontends/af9033.c b/drivers/media/dvb-frontends/af9033.c index a777b4b944eb..11f1555e66dc 100644 --- a/drivers/media/dvb-frontends/af9033.c +++ b/drivers/media/dvb-frontends/af9033.c @@ -21,6 +21,9 @@ #include "af9033_priv.h" +/* Max transfer size done by I2C transfer functions */ +#define MAX_XFER_SIZE 64 + struct af9033_state { struct i2c_adapter *i2c; struct dvb_frontend fe; @@ -40,7 +43,7 @@ static int af9033_wr_regs(struct af9033_state *state, u32 reg, const u8 *val, int len) { int ret; - u8 buf[3 + len]; + u8 buf[MAX_XFER_SIZE]; struct i2c_msg msg[1] = { { .addr = state->cfg.i2c_addr, @@ -50,6 +53,13 @@ static int af9033_wr_regs(struct af9033_state *state, u32 reg, const u8 *val, } }; + if (3 + len > sizeof(buf)) { + dev_warn(&state->i2c->dev, +"%s: i2c wr reg=%04x: len=%d is too big!\n", +KBUILD_MODNAME, reg, len); + return -EINVAL; + } + buf[0] = (reg >> 16) & 0xff; buf[1] = (reg >> 8) & 0xff; buf[2] = (reg >> 0) & 0xff; @@ -161,7 +171,14 @@ static int af9033_wr_reg_val_tab(struct af9033_state *state, const
Re: [RFC] [PATCH] media: marvell-ccic: use devm to release clk
On Tue, 5 Nov 2013 17:33:16 +0800 lbyang wrote: > This patch uses devm to release the clks instead of releasing > manually. > And it adds enable/disable mipi_clk when getting its rate. I can't really test this, so I'll have to assume it works :) Acked-by: Jonathan Corbet However: it seems that crediting Uwe Kleine-König with a Reported-by would be the right thing to do. Thanks, jon -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] media: marvell-ccic: drop resource free in driver remove
On Tue, 5 Nov 2013 11:21:08 +0800 lbyang wrote: > From: Libin Yang > Date: Tue, 5 Nov 2013 10:18:15 +0800 > Subject: [PATCH] marvell-ccic: drop resource free in driver remove > > The mmp-driver is using devm_* to allocate the resource. The old > resource release methods are not appropriate here. Acked-by: Jonathan Corbet jon -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 17/29] [media] av7110_hw: Don't use dynamic static allocation
Dynamic static allocation is evil, as Kernel stack is too low, and compilation complains about it on some archs: drivers/media/pci/ttpci/av7110_hw.c:510:1: warning: 'av7110_fw_cmd' uses dynamic stack allocation [enabled by default] Instead, let's enforce a limit for the buffer. In the specific case of this driver, the maximum fw command size is 6 + 2, as checked using: $ git grep -A1 av7110_fw_cmd drivers/media/pci/ttpci/ So, use 8 for the buffer size. Signed-off-by: Mauro Carvalho Chehab --- drivers/media/pci/ttpci/av7110_hw.c | 19 +-- 1 file changed, 17 insertions(+), 2 deletions(-) diff --git a/drivers/media/pci/ttpci/av7110_hw.c b/drivers/media/pci/ttpci/av7110_hw.c index f1cbfe526989..6299d5dadb82 100644 --- a/drivers/media/pci/ttpci/av7110_hw.c +++ b/drivers/media/pci/ttpci/av7110_hw.c @@ -22,7 +22,7 @@ * Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA. * Or, point your browser to http://www.gnu.org/copyleft/gpl.html * - * the project's page is at http://www.linuxtv.org/ + * the project's page is at http://www.linuxtv.org/ */ /* for debugging ARM communication: */ @@ -40,6 +40,14 @@ #define _NOHANDSHAKE +/* + * Max transfer size done by av7110_fw_cmd() + * + * The maximum size passed to this function is 6 bytes. The buffer also + * uses two additional ones for type and size. So, 8 bytes is enough. + */ +#define MAX_XFER_SIZE 8 + / * DEBI functions / @@ -488,11 +496,18 @@ static int av7110_send_fw_cmd(struct av7110 *av7110, u16* buf, int length) int av7110_fw_cmd(struct av7110 *av7110, int type, int com, int num, ...) { va_list args; - u16 buf[num + 2]; + u16 buf[MAX_XFER_SIZE]; int i, ret; // dprintk(4, "%p\n", av7110); + if (2 + num > sizeof(buf)) { + printk(KERN_WARNING + "%s: %s len=%d is too big!\n", + KBUILD_MODNAME, __func__, num); + return -EINVAL; + } + buf[0] = ((type << 8) | com); buf[1] = num; -- 1.8.3.1 -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 00/29] Fix errors/warnings with allmodconfig/allyesconfig on non-x86 archs
Hi Mauro, For all patches except 22 (which got a newer revision): Reviewed-by: Hans Verkuil Regards, Hans On 11/05/13 11:01, Mauro Carvalho Chehab wrote: > To be sure that we're not introducing compilation regressions on media, I'm > now > using ktest to check for errors/warnings. > > My current setup is cross-building on several architectures: > alpha, arm, avr32, cris (64), frv, i386, ia64, m32r, m68k, mips, > openrisc, parisc, s390, sh, sparc, sparc64, uml, x86_64 > > I tried to enable a few other archs: > blackfin, cris (32), powerpc (32, 64), tile, xtensa > > but they fail to compile with allyesconfig due to non-media related issues. > > I'm still unsure about how often I'll be doing it, I intend to run it at least > by the end of the subsystem merge window (by -rc6 or -rc7), and fix the > issues found there. > > V3: contains fixes for the feedbacks received: > - I2C client driver now returns -EINVAL; > - I2C adapter drivers now returns -EOPNOSUPP; > - a macro was added for the buffers; > - check for failure on allocation at v4l2-async was added; > - Used iguanair code from Sean, instead of my code; > - Most buffer sizes changed to 64 bytes, to match max URB > control size. > > Mauro Carvalho Chehab (28): > [media] tda9887: remove an warning when compiling for alpha > [media] radio-shark: remove a warning when CONFIG_PM is not defined > [media] zoran: don't build it on alpha > [media] cx18: struct i2c_client is too big for stack > [media] tef6862: fix warning on avr32 arch > [media] platform drivers: Fix build on frv arch > [media] radio-si470x-i2c: fix a warning on ia64 > [media] rc: Fir warnings on m68k arch > [media] uvc/lirc_serial: Fix some warnings on parisc arch > [media] s5h1420: Don't use dynamic static allocation > [media] dvb-frontends: Don't use dynamic static allocation > [media] dvb-frontends: Don't use dynamic static allocation > [media] stb0899_drv: Don't use dynamic static allocation > [media] stv0367: Don't use dynamic static allocation > [media] stv090x: Don't use dynamic static allocation > [media] av7110_hw: Don't use dynamic static allocation > [media] tuners: Don't use dynamic static allocation > [media] tuner-xc2028: Don't use dynamic static allocation > [media] cimax2: Don't use dynamic static allocation > [media] v4l2-async: Don't use dynamic static allocation > [media] cxusb: Don't use dynamic static allocation > [media] dibusb-common: Don't use dynamic static allocation > [media] dw2102: Don't use dynamic static allocation > [media] af9015: Don't use dynamic static allocation > [media] af9035: Don't use dynamic static allocation > [media] mxl111sf: Don't use dynamic static allocation > [media] lirc_zilog: Don't use dynamic static allocation > [media] cx18: disable compilation on frv arch > > Sean Young (1): > [media] iguanair: simplify calculation of carrier delay cycles > > drivers/media/dvb-frontends/af9013.c | 12 +++- > drivers/media/dvb-frontends/af9033.c | 21 ++- > drivers/media/dvb-frontends/bcm3510.c | 15 - > drivers/media/dvb-frontends/cxd2820r_core.c | 21 ++- > drivers/media/dvb-frontends/itd1000.c | 13 +++- > drivers/media/dvb-frontends/mt312.c | 10 ++- > drivers/media/dvb-frontends/nxt200x.c | 11 +++- > drivers/media/dvb-frontends/rtl2830.c | 12 +++- > drivers/media/dvb-frontends/rtl2832.c | 12 +++- > drivers/media/dvb-frontends/s5h1420.c | 9 ++- > drivers/media/dvb-frontends/stb0899_drv.c | 12 +++- > drivers/media/dvb-frontends/stb6100.c | 11 +++- > drivers/media/dvb-frontends/stv0367.c | 13 +++- > drivers/media/dvb-frontends/stv090x.c | 12 +++- > drivers/media/dvb-frontends/stv6110.c | 12 +++- > drivers/media/dvb-frontends/stv6110x.c| 13 +++- > drivers/media/dvb-frontends/tda10071.c| 21 ++- > drivers/media/dvb-frontends/tda18271c2dd.c| 14 - > drivers/media/dvb-frontends/zl10039.c | 12 +++- > drivers/media/pci/cx18/Kconfig| 1 + > drivers/media/pci/cx18/cx18-driver.c | 20 +++--- > drivers/media/pci/cx23885/cimax2.c| 13 +++- > drivers/media/pci/ttpci/av7110_hw.c | 19 +- > drivers/media/pci/zoran/Kconfig | 1 + > drivers/media/platform/soc_camera/rcar_vin.c | 1 + > drivers/media/radio/radio-shark.c | 2 + > drivers/media/radio/radio-shark2.c| 2 + > drivers/media/radio/si470x/radio-si470x-i2c.c | 4 +- > drivers/media/radio/tef6862.c | 20 +++--- > drivers/media/rc/fintek-cir.h | 4 +- > drivers/media/rc/iguanair.c | 22 ++- > drivers/media/rc/nuvoton-cir.h| 4 +- > drivers/media/tuners/e4000.c | 21 ++- > drivers/media/tuners/fc2580.c
[PATCH v3 25/29] [media] af9015: Don't use dynamic static allocation
Dynamic static allocation is evil, as Kernel stack is too low, and compilation complains about it on some archs: drivers/media/usb/dvb-usb-v2/af9015.c:433:1: warning: 'af9015_eeprom_hash' uses dynamic stack allocation [enabled by default] In this specific case, it is a gcc bug, as the size is a const, but it is easy to just change it from const to a #define, getting rid of the gcc warning. Signed-off-by: Mauro Carvalho Chehab --- drivers/media/usb/dvb-usb-v2/af9015.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/media/usb/dvb-usb-v2/af9015.c b/drivers/media/usb/dvb-usb-v2/af9015.c index d556042cf312..da47d2392f2a 100644 --- a/drivers/media/usb/dvb-usb-v2/af9015.c +++ b/drivers/media/usb/dvb-usb-v2/af9015.c @@ -397,12 +397,13 @@ error: return ret; } +#define AF9015_EEPROM_SIZE 256 + /* hash (and dump) eeprom */ static int af9015_eeprom_hash(struct dvb_usb_device *d) { struct af9015_state *state = d_to_priv(d); int ret, i; - static const unsigned int AF9015_EEPROM_SIZE = 256; u8 buf[AF9015_EEPROM_SIZE]; struct req_t req = {READ_I2C, AF9015_I2C_EEPROM, 0, 0, 1, 1, NULL}; -- 1.8.3.1 -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv2 22/29] v4l2-async: Don't use dynamic static allocation
On 11/05/13 14:16, Mauro Carvalho Chehab wrote: > New patch enclosed. Please reply with your reviewed-by if you're ok with > it. Reviewed-by: Hans Verkuil 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: [REVIEW PATCH 6/9] si4713 : Added the USB driver for Si4713
Dinesh On Tue, Oct 15, 2013 at 11:24 AM, Dinesh Ram wrote: > This is the USB driver for the Silicon Labs development board. > It contains the Si4713 FM transmitter chip. > I tried this driver again. The system attempts to probe the device but it fails because the product revision read out of the USB device is wrong. [ 220.855158] usb 2-1.3.3: new full-speed USB device number 10 using ehci-pci [ 220.949677] usb 2-1.3.3: New USB device found, idVendor=10c4, idProduct=8244 [ 220.949683] usb 2-1.3.3: New USB device strings: Mfr=1, Product=2, SerialNumber=3 [ 220.949688] usb 2-1.3.3: Product: Si47xx Baseboard [ 220.949691] usb 2-1.3.3: Manufacturer: SILICON LABORATORIES INC. [ 220.949695] usb 2-1.3.3: SerialNumber: CBDA8-00-0 [ 220.950157] usbhid 2-1.3.3:1.0: couldn't find an input interrupt endpoint [ 1014.981012] radio-usb-si4713 2-1.3.3:1.0: Si4713 development board discovered: (10C4:8244) [ 1015.870984] si4713 12-0063: IRQ not configured. Using timeouts. [ 1015.943551] si4713 12-0063: Invalid product number <<< Here is the code without modification [ 1015.943556] si4713 12-0063: Failed to probe device information. [ 1015.943568] si4713: probe of 12-0063 failed with error -22 [ 1015.943613] radio-usb-si4713 2-1.3.3:1.0: cannot get v4l2 subdevice [ 1015.943672] usbcore: registered new interface driver radio-usb-si4713 [ 1274.419987] perf samples too long (2504 > 2500), lowering kernel.perf_event_max_sample_rate to 5 [ 1308.851059] usbcore: deregistering interface driver radio-usb-si4713 [ 1500.478308] radio-usb-si4713 2-1.3.3:1.0: Si4713 development board discovered: (10C4:8244) [ 1500.612240] si4713 12-0063: IRQ not configured. Using timeouts. [ 1500.683489] si4713 12-0063: Invalid product number 0x15 <<< Here it prints the PN read [ 1500.683495] si4713 12-0063: Failed to probe device information. [ 1500.683509] si4713: probe of 12-0063 failed with error -22 [ 1500.683558] radio-usb-si4713 2-1.3.3:1.0: cannot get v4l2 subdevice [ 1500.683624] usbcore: registered new interface driver radio-usb-si4713 Here is simple diff of what I used to print the PN value: diff --git a/drivers/media/radio/si4713/si4713.c b/drivers/media/radio/si4713/si4713.c index aadecb5..ee53584 100644 --- a/drivers/media/radio/si4713/si4713.c +++ b/drivers/media/radio/si4713/si4713.c @@ -464,7 +464,7 @@ static int si4713_checkrev(struct si4713_device *sdev) v4l2_info(&sdev->sd, "chip found @ 0x%02x (%s)\n", client->addr << 1, client->adapter->name); } else { - v4l2_err(&sdev->sd, "Invalid product number\n"); + v4l2_err(&sdev->sd, "Invalid product number 0x%X\n", resp[1]); rval = -EINVAL; } return rval; It is expected to be 0x0D instead of 0x15, if I am not mistaken. > Signed-off-by: Dinesh Ram > --- > drivers/media/radio/si4713/Kconfig| 15 + > drivers/media/radio/si4713/Makefile |1 + > drivers/media/radio/si4713/radio-usb-si4713.c | 540 > + > 3 files changed, 556 insertions(+) > create mode 100644 drivers/media/radio/si4713/radio-usb-si4713.c > > diff --git a/drivers/media/radio/si4713/Kconfig > b/drivers/media/radio/si4713/Kconfig > index ec640b8..a7c3ba8 100644 > --- a/drivers/media/radio/si4713/Kconfig > +++ b/drivers/media/radio/si4713/Kconfig > @@ -1,3 +1,18 @@ > +config USB_SI4713 > + tristate "Silicon Labs Si4713 FM Radio Transmitter support with USB" > + depends on USB && RADIO_SI4713 > + select SI4713 > + ---help--- > + This is a driver for USB devices with the Silicon Labs SI4713 > + chip. Currently these devices are known to work. > + - 10c4:8244: Silicon Labs FM Transmitter USB device. > + > + Say Y here if you want to connect this type of radio to your > + computer's USB port. > + > + To compile this driver as a module, choose M here: the > + module will be called radio-usb-si4713. > + > config PLATFORM_SI4713 > tristate "Silicon Labs Si4713 FM Radio Transmitter support with I2C" > depends on I2C && RADIO_SI4713 > diff --git a/drivers/media/radio/si4713/Makefile > b/drivers/media/radio/si4713/Makefile > index 9d0bd0e..6524674 100644 > --- a/drivers/media/radio/si4713/Makefile > +++ b/drivers/media/radio/si4713/Makefile > @@ -3,5 +3,6 @@ > # > > obj-$(CONFIG_I2C_SI4713) += si4713.o > +obj-$(CONFIG_USB_SI4713) += radio-usb-si4713.o > obj-$(CONFIG_PLATFORM_SI4713) += radio-platform-si4713.o > > diff --git a/drivers/media/radio/si4713/radio-usb-si4713.c > b/drivers/media/radio/si4713/radio-usb-si4713.c > new file mode 100644 > index 000..a75e2c8 > --- /dev/null > +++ b/drivers/media/radio/si4713/radio-usb-si4713.c > @@ -0,0 +1,540 @@ > +/* > + * Copyright 2013 Cisco Systems, Inc. and/or its affiliates. > + * All rights reserved. > + * > + * This program is free software; you may redistribute it and/or modify > + * it under the terms
Re: [REVIEW PATCH 8/9] si4713: move supply list to si4713_platform_data
On Tue, Nov 5, 2013 at 4:38 AM, Hans Verkuil wrote: > Hi, > > On 11/04/13 15:07, edubez...@gmail.com wrote: >> Hi, >> >> On Tue, Oct 15, 2013 at 11:24 AM, Dinesh Ram wrote: >>> The supply list is needed by the platform driver, but not by the usb driver. >>> So this information belongs to the platform data and should not be hardcoded >>> in the subdevice driver. >>> >>> Signed-off-by: Hans Verkuil >> >> Dinesh, could you please sign this patch too? >> >>> --- >>> arch/arm/mach-omap2/board-rx51-peripherals.c |7 >>> drivers/media/radio/si4713/si4713.c | 52 >>> +- >>> drivers/media/radio/si4713/si4713.h |3 +- >>> include/media/si4713.h |2 + >>> 4 files changed, 37 insertions(+), 27 deletions(-) >>> >>> diff --git a/arch/arm/mach-omap2/board-rx51-peripherals.c >>> b/arch/arm/mach-omap2/board-rx51-peripherals.c >>> index f6fe388..eae73f7 100644 >>> --- a/arch/arm/mach-omap2/board-rx51-peripherals.c >>> +++ b/arch/arm/mach-omap2/board-rx51-peripherals.c >>> @@ -776,7 +776,14 @@ static struct regulator_init_data rx51_vintdig = { >>> }, >>> }; >>> >>> +static const char * const si4713_supply_names[SI4713_NUM_SUPPLIES] = { >> >> This patch produces the following compilation error: >> arch/arm/mach-omap2/board-rx51-peripherals.c:779:47: error: >> 'SI4713_NUM_SUPPLIES' undeclared here (not in a function) > > Hmm, I thought I had compile-tested this, apparently not. Does it compile if > you just remove SI4713_NUM_SUPPLIES? It's not necessary here. Yes, it does compile. I just tried compiling testing with omap2plus_defconfig, that is what I use to boot n900 too. > > Regards, > > Hans > >> arch/arm/mach-omap2/board-rx51-peripherals.c:785:14: error: bit-field >> '' width not an integer constant >> arch/arm/mach-omap2/board-rx51-peripherals.c:779:27: warning: >> 'si4713_supply_names' defined but not used [-Wunused-variable] >> make[1]: *** [arch/arm/mach-omap2/board-rx51-peripherals.o] Error 1 >> make: *** [arch/arm/mach-omap2] Error 2 >> make: *** Waiting for unfinished jobs >> >> >>> + "vio", >>> + "vdd", >>> +}; >>> + >>> static struct si4713_platform_data rx51_si4713_i2c_data >>> __initdata_or_module = { >>> + .supplies = ARRAY_SIZE(si4713_supply_names), >>> + .supply_names = si4713_supply_names, >>> .gpio_reset = RX51_FMTX_RESET_GPIO, >>> }; >>> >>> diff --git a/drivers/media/radio/si4713/si4713.c >>> b/drivers/media/radio/si4713/si4713.c >>> index d297a5b..920dfa5 100644 >>> --- a/drivers/media/radio/si4713/si4713.c >>> +++ b/drivers/media/radio/si4713/si4713.c >>> @@ -44,11 +44,6 @@ MODULE_AUTHOR("Eduardo Valentin >>> "); >>> MODULE_DESCRIPTION("I2C driver for Si4713 FM Radio Transmitter"); >>> MODULE_VERSION("0.0.1"); >>> >>> -static const char *si4713_supply_names[SI4713_NUM_SUPPLIES] = { >>> - "vio", >>> - "vdd", >>> -}; >>> - >>> #define DEFAULT_RDS_PI 0x00 >>> #define DEFAULT_RDS_PTY0x00 >>> #define DEFAULT_RDS_DEVIATION 0x00C8 >>> @@ -368,11 +363,12 @@ static int si4713_powerup(struct si4713_device *sdev) >>> if (sdev->power_state) >>> return 0; >>> >>> - err = regulator_bulk_enable(ARRAY_SIZE(sdev->supplies), >>> - sdev->supplies); >>> - if (err) { >>> - v4l2_err(&sdev->sd, "Failed to enable supplies: %d\n", err); >>> - return err; >>> + if (sdev->supplies) { >>> + err = regulator_bulk_enable(sdev->supplies, >>> sdev->supply_data); >>> + if (err) { >>> + v4l2_err(&sdev->sd, "Failed to enable supplies: >>> %d\n", err); >>> + return err; >>> + } >>> } >>> if (gpio_is_valid(sdev->gpio_reset)) { >>> udelay(50); >>> @@ -396,11 +392,12 @@ static int si4713_powerup(struct si4713_device *sdev) >>> if (client->irq) >>> err = si4713_write_property(sdev, SI4713_GPO_IEN, >>> SI4713_STC_INT | >>> SI4713_CTS); >>> - } else { >>> - if (gpio_is_valid(sdev->gpio_reset)) >>> - gpio_set_value(sdev->gpio_reset, 0); >>> - err = regulator_bulk_disable(ARRAY_SIZE(sdev->supplies), >>> -sdev->supplies); >>> + return err; >>> + } >>> + if (gpio_is_valid(sdev->gpio_reset)) >>> + gpio_set_value(sdev->gpio_reset, 0); >>> + if (sdev->supplies) { >>> + err = regulator_bulk_disable(sdev->supplies, >>> sdev->supply_data); >>> if (err) >>> v4l2_err(&sdev->sd, >>> "Failed to disable supplies: %d\n", err); >>> @@ -432,11 +429,13 @@ static int si4713_powerdown(struct si4713_device
Re: [PATCH v11 03/12] [media] exynos5-fimc-is: Add common driver header files
Hi Sakari, Thank you for the review. On Tue, Nov 5, 2013 at 6:21 PM, Sakari Ailus wrote: > Hi Arun, > > On Tue, Nov 05, 2013 at 11:42:34AM +0530, Arun Kumar K wrote: >> This patch adds all the common header files used by the fimc-is >> driver. It includes the commands for interfacing with the firmware >> and error codes from IS firmware, metadata and command parameter >> definitions. >> >> Signed-off-by: Arun Kumar K >> Signed-off-by: Kilyeon Im >> Reviewed-by: Sylwester Nawrocki >> --- >> drivers/media/platform/exynos5-is/fimc-is-cmd.h| 187 >> drivers/media/platform/exynos5-is/fimc-is-err.h| 257 + >> .../media/platform/exynos5-is/fimc-is-metadata.h | 767 + >> drivers/media/platform/exynos5-is/fimc-is-param.h | 1159 >> >> 4 files changed, 2370 insertions(+) >> create mode 100644 drivers/media/platform/exynos5-is/fimc-is-cmd.h >> create mode 100644 drivers/media/platform/exynos5-is/fimc-is-err.h >> create mode 100644 drivers/media/platform/exynos5-is/fimc-is-metadata.h >> create mode 100644 drivers/media/platform/exynos5-is/fimc-is-param.h >> >> diff --git a/drivers/media/platform/exynos5-is/fimc-is-cmd.h >> b/drivers/media/platform/exynos5-is/fimc-is-cmd.h >> new file mode 100644 >> index 000..6250280 >> --- /dev/null >> +++ b/drivers/media/platform/exynos5-is/fimc-is-cmd.h >> @@ -0,0 +1,187 @@ >> +/* [snip] >> +struct is_common_reg { >> + u32 hicmd; >> + u32 hic_sensorid; >> + u32 hic_param[4]; >> + >> + u32 reserved1[3]; >> + >> + u32 ihcmd_iflag; >> + u32 ihcmd; >> + u32 ihc_sensorid; >> + u32 ihc_param[4]; >> + >> + u32 reserved2[3]; >> + >> + u32 isp_bayer_iflag; >> + u32 isp_bayer_sensor_id; >> + u32 isp_bayer_param[2]; >> + >> + u32 reserved3[4]; >> + >> + u32 scc_iflag; >> + u32 scc_sensor_id; >> + u32 scc_param[3]; >> + >> + u32 reserved4[3]; >> + >> + u32 dnr_iflag; >> + u32 dnr_sensor_id; >> + u32 dnr_param[2]; >> + >> + u32 reserved5[4]; >> + >> + u32 scp_iflag; >> + u32 scp_sensor_id; >> + u32 scp_param[3]; >> + >> + u32 reserved6[1]; >> + >> + u32 isp_yuv_iflag; >> + u32 isp_yuv_sensor_id; >> + u32 isp_yuv_param[2]; >> + >> + u32 reserved7[1]; >> + >> + u32 shot_iflag; >> + u32 shot_sensor_id; >> + u32 shot_param[2]; >> + >> + u32 reserved8[1]; >> + >> + u32 meta_iflag; >> + u32 meta_sensor_id; >> + u32 meta_param1; >> + >> + u32 reserved9[1]; >> + >> + u32 fcount; > > If these structs define an interface that's not used by the driver only it > might be a good idea to use __packed to ensure no padding is added. > The same structure is used as is in the firmware code and so it is retained in the driver. >> +}; >> + >> +struct is_mcuctl_reg { >> + u32 mcuctl; >> + u32 bboar; >> + >> + u32 intgr0; >> + u32 intcr0; >> + u32 intmr0; >> + u32 intsr0; >> + u32 intmsr0; >> + >> + u32 intgr1; >> + u32 intcr1; >> + u32 intmr1; >> + u32 intsr1; >> + u32 intmsr1; >> + >> + u32 intcr2; >> + u32 intmr2; >> + u32 intsr2; >> + u32 intmsr2; >> + >> + u32 gpoctrl; >> + u32 cpoenctlr; >> + u32 gpictlr; >> + >> + u32 pad[0xD]; >> + >> + struct is_common_reg common_reg; >> +}; >> +#endif > ... >> diff --git a/drivers/media/platform/exynos5-is/fimc-is-metadata.h >> b/drivers/media/platform/exynos5-is/fimc-is-metadata.h >> new file mode 100644 >> index 000..02367c4 >> --- /dev/null >> +++ b/drivers/media/platform/exynos5-is/fimc-is-metadata.h >> @@ -0,0 +1,767 @@ >> +/* >> + * Samsung EXYNOS5 FIMC-IS (Imaging Subsystem) driver >> + * >> + * Copyright (C) 2013 Samsung Electronics Co., Ltd. >> + * Kil-yeon Lim >> + * Arun Kumar K >> + * >> + * 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. >> + */ >> + >> +#ifndef FIMC_IS_METADATA_H_ >> +#define FIMC_IS_METADATA_H_ >> + >> +struct rational { >> + uint32_t num; >> + uint32_t den; >> +}; >> + >> +#define CAMERA2_MAX_AVAILABLE_MODE 21 >> +#define CAMERA2_MAX_FACES16 >> + >> +/* >> + * Controls/dynamic metadata >> + */ >> + >> +enum metadata_mode { >> + METADATA_MODE_NONE, >> + METADATA_MODE_FULL >> +}; >> + >> +struct camera2_request_ctl { >> + uint32_tid; >> + enum metadata_mode metadatamode; >> + uint8_t outputstreams[16]; >> + uint32_tframecount; >> +}; >> + >> +struct camera2_request_dm { >> + uint32_tid; >> + enum metadata_mode metadatamode; >> + uint32_tframecount; >> +}; >> + >> + >> + >> +enum optical_stabilization_mode { >> + OPTICAL_STABILIZATION_MODE_OFF, >> + OPTICAL_STABILIZATION_MODE_ON >> +}; >> + >> +enum lens_facing { >> + LENS_FACING_B
Re: [PATCHv2 22/29] v4l2-async: Don't use dynamic static allocation
Em Tue, 05 Nov 2013 13:35:49 +0100 Hans Verkuil escreveu: > On 11/05/13 13:03, Mauro Carvalho Chehab wrote: > > Em Tue, 05 Nov 2013 12:42:23 +0100 > > Sylwester Nawrocki escreveu: > > > >> On 05/11/13 12:36, Mauro Carvalho Chehab wrote: > > diff --git a/drivers/media/v4l2-core/v4l2-async.c > > b/drivers/media/v4l2-core/v4l2-async.c > >>> index c85d69da35bd..071596869036 100644 > >>> --- a/drivers/media/v4l2-core/v4l2-async.c > >>> +++ b/drivers/media/v4l2-core/v4l2-async.c > >>> @@ -189,12 +189,14 @@ void v4l2_async_notifier_unregister(struct > >>> v4l2_async_notifier *notifier) > >>> struct v4l2_subdev *sd, *tmp; > >>> unsigned int notif_n_subdev = notifier->num_subdevs; > >>> unsigned int n_subdev = min(notif_n_subdev, V4L2_MAX_SUBDEVS); > >>> - struct device *dev[n_subdev]; > >>> + struct device **dev; > >>> int i = 0; > >>> > >>> if (!notifier->v4l2_dev) > >>> return; > >>> > >>> + dev = kmalloc(sizeof(*dev) * n_subdev, GFP_KERNEL); > >>> + > > > > No check for dev == NULL? > >>> Well, what should be done in this case? > >>> > >>> We could do the changes below: > >>> > >>> void v4l2_async_notifier_unregister(struct v4l2_async_notifier *notifier) > >>> { > >>> struct v4l2_subdev *sd, *tmp; > >>> unsigned int notif_n_subdev = notifier->num_subdevs; > >>> unsigned int n_subdev = min(notif_n_subdev, V4L2_MAX_SUBDEVS); > >>> - struct device *dev[n_subdev]; > >>> + struct device **dev; > >>> int i = 0; > >>> > >>> if (!notifier->v4l2_dev) > >>> return; > >>> > >>> + dev = kmalloc(sizeof(*dev) * n_subdev, GFP_KERNEL); > >>> + if (!dev) { > >>> + WARN_ON(true); > >>> + return; > >>> + } > >>> + > >>> mutex_lock(&list_lock); > >>> > >>> list_del(¬ifier->list); > >>> > >>> list_for_each_entry_safe(sd, tmp, ¬ifier->done, async_list) { > >>> dev[i] = get_device(sd->dev); > >>> > >>> v4l2_async_cleanup(sd); > >>> > >>> /* If we handled USB devices, we'd have to lock the > >>> parent too */ > >>> device_release_driver(dev[i++]); > >>> > >>> if (notifier->unbind) > >>> notifier->unbind(notifier, sd, sd->asd); > >>> } > >>> > >>> mutex_unlock(&list_lock); > >>> > >>> while (i--) { > >>> struct device *d = dev[i]; > >>> > >>> if (d && device_attach(d) < 0) { > >>> const char *name = "(none)"; > >>> int lock = device_trylock(d); > >>> > >>> if (lock && d->driver) > >>> name = d->driver->name; > >>> dev_err(d, "Failed to re-probe to %s\n", name); > >>> if (lock) > >>> device_unlock(d); > >>> } > >>> put_device(d); > >>> } > >>> + kfree(dev); > >>> > >>> notifier->v4l2_dev = NULL; > >>> > >>> /* > >>> * Don't care about the waiting list, it is initialised and > >>> populated > >>> * upon notifier registration. > >>> */ > >>> } > >>> EXPORT_SYMBOL(v4l2_async_notifier_unregister); > >>> > >>> But I suspect that this will cause an OOPS anyway, as the device will be > >>> only half-removed. So, it would likely OOPS at device removal or if the > >>> device got probed again, at probing time. > >>> > >>> So, IMHO, we should have at least a WARN_ON() for this case. > >>> > >>> Do you have a better idea? > >> > >> This is how Guennadi's patch looked like when it used dynamic allocation: > >> > >> http://www.spinics.net/lists/linux-sh/msg18194.html > > > > Thanks for the tip! > > > > The following patch should do the trick (generated with -U10, in order > > to show the entire function): > > > > [PATCHv3] v4l2-async: Don't use dynamic static allocation > > > > Dynamic static allocation is evil, as Kernel stack is too low, and > > compilation complains about it on some archs: > > > > drivers/media/v4l2-core/v4l2-async.c:238:1: warning: > > 'v4l2_async_notifier_unregister' uses dynamic stack allocation [enabled by > > default] > > > > Instead, let's enforce a limit for the buffer. > > > > In this specific case, there's a hard limit imposed by V4L2_MAX_SUBDEVS, > > with is currently 128. That means that the buffer size can be up to > > 128x8 = 1024 bytes (on a 64bits kernel), with is too big for stack. > > > > Worse than that, someone could increase it and cause real troubles. > > > > So, let's use dynamically allocated data, instead. > > > > Signed-off-by: Mauro Carvalho Chehab > > Cc: Guennadi Liakhovetski > > > > diff --git a/drivers/media/v4l2-core/v4l2-async.c > > b/driver
[PATCH v3 08/29] [media] radio-si470x-i2c: fix a warning on ia64
on ia64, those warnings appear: drivers/media/radio/si470x/radio-si470x-i2c.c:470:12: warning: 'si470x_i2c_suspend' defined but not used [-Wunused-function] drivers/media/radio/si470x/radio-si470x-i2c.c:487:12: warning: 'si470x_i2c_resume' defined but not used [-Wunused-function] They're caused because the PM logic uses this define: #define SET_SYSTEM_SLEEP_PM_OPS() With is only defined for CONFIG_PM_SLEEP. So, change the logic there to test for CONFIG_PM_SLEEP, instead of CONFIG_PM. Signed-off-by: Mauro Carvalho Chehab --- drivers/media/radio/si470x/radio-si470x-i2c.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/media/radio/si470x/radio-si470x-i2c.c b/drivers/media/radio/si470x/radio-si470x-i2c.c index e5fc9acd0c4f..2a497c80c77f 100644 --- a/drivers/media/radio/si470x/radio-si470x-i2c.c +++ b/drivers/media/radio/si470x/radio-si470x-i2c.c @@ -463,7 +463,7 @@ static int si470x_i2c_remove(struct i2c_client *client) } -#ifdef CONFIG_PM +#ifdef CONFIG_PM_SLEEP /* * si470x_i2c_suspend - suspend the device */ @@ -509,7 +509,7 @@ static struct i2c_driver si470x_i2c_driver = { .driver = { .name = "si470x", .owner = THIS_MODULE, -#ifdef CONFIG_PM +#ifdef CONFIG_PM_SLEEP .pm = &si470x_i2c_pm, #endif }, -- 1.8.3.1 -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 29/29] [media] cx18: disable compilation on frv arch
This driver produces a lot of errors on this arch: In file included from drivers/media/pci/cx18/cx18-driver.c:26:0: /drivers/media/pci/cx18/cx18-io.h: In function 'cx18_raw_readl': drivers/media/pci/cx18/cx18-io.h:40:2: warning: passing argument 1 of '__builtin_read32' discards 'const' qualifier from pointer target type [enabled by default] arch/frv/include/asm/mb-regs.h:24:15: note: expected 'volatile void *' but argument is of type 'const void *' ... While this is not fixed, just disable it. NOTE: I'll likely remove this patch from the final series, as this should be fixed inside gcc code. Signed-off-by: Mauro Carvalho Chehab --- drivers/media/pci/cx18/Kconfig | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/media/pci/cx18/Kconfig b/drivers/media/pci/cx18/Kconfig index c675b83c43a9..10e6bc72c460 100644 --- a/drivers/media/pci/cx18/Kconfig +++ b/drivers/media/pci/cx18/Kconfig @@ -1,6 +1,7 @@ config VIDEO_CX18 tristate "Conexant cx23418 MPEG encoder support" depends on VIDEO_V4L2 && DVB_CORE && PCI && I2C + depends on !FRV select I2C_ALGOBIT select VIDEOBUF_VMALLOC depends on RC_CORE -- 1.8.3.1 -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 23/29] [media] dibusb-common: Don't use dynamic static allocation
Dynamic static allocation is evil, as Kernel stack is too low, and compilation complains about it on some archs: drivers/media/usb/dvb-usb/dibusb-common.c:124:1: warning: 'dibusb_i2c_msg' uses dynamic stack allocation [enabled by default] Instead, let's enforce a limit for the buffer to be the max size of a control URB payload data (64 bytes). Signed-off-by: Mauro Carvalho Chehab --- drivers/media/usb/dvb-usb/dibusb-common.c | 10 +- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/drivers/media/usb/dvb-usb/dibusb-common.c b/drivers/media/usb/dvb-usb/dibusb-common.c index c2dded92f1d3..6d68af0c49c8 100644 --- a/drivers/media/usb/dvb-usb/dibusb-common.c +++ b/drivers/media/usb/dvb-usb/dibusb-common.c @@ -12,6 +12,9 @@ #include #include "dibusb.h" +/* Max transfer size done by I2C transfer functions */ +#define MAX_XFER_SIZE 64 + static int debug; module_param(debug, int, 0644); MODULE_PARM_DESC(debug, "set debugging level (1=info (|-able))." DVB_USB_DEBUG_STATUS); @@ -105,11 +108,16 @@ EXPORT_SYMBOL(dibusb2_0_power_ctrl); static int dibusb_i2c_msg(struct dvb_usb_device *d, u8 addr, u8 *wbuf, u16 wlen, u8 *rbuf, u16 rlen) { - u8 sndbuf[wlen+4]; /* lead(1) devaddr,direction(1) addr(2) data(wlen) (len(2) (when reading)) */ + u8 sndbuf[MAX_XFER_SIZE]; /* lead(1) devaddr,direction(1) addr(2) data(wlen) (len(2) (when reading)) */ /* write only ? */ int wo = (rbuf == NULL || rlen == 0), len = 2 + wlen + (wo ? 0 : 2); + if (4 + wlen > sizeof(sndbuf)) { + warn("i2c wr: len=%d is too big!\n", wlen); + return -EOPNOTSUPP; + } + sndbuf[0] = wo ? DIBUSB_REQ_I2C_WRITE : DIBUSB_REQ_I2C_READ; sndbuf[1] = (addr << 1) | (wo ? 0 : 1); -- 1.8.3.1 -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 16/29] [media] stv090x: Don't use dynamic static allocation
Dynamic static allocation is evil, as Kernel stack is too low, and compilation complains about it on some archs: drivers/media/dvb-frontends/stv090x.c:750:1: warning: 'stv090x_write_regs.constprop.6' uses dynamic stack allocation [enabled by default] Instead, let's enforce a limit for the buffer. Considering that I2C transfers are generally limited, and that devices used on USB has a max data length of 64 bytes for the control URBs. So, it seem safe to use 64 bytes as the hard limit for all those devices. On most cases, the limit is a way lower than that, but this limit is small enough to not affect the Kernel stack, and it is a no brain limit, as using smaller ones would require to either carefully each driver or to take a look on each datasheet. Signed-off-by: Mauro Carvalho Chehab --- drivers/media/dvb-frontends/stv090x.c | 12 +++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/drivers/media/dvb-frontends/stv090x.c b/drivers/media/dvb-frontends/stv090x.c index 56d470ad5a82..23e872f84742 100644 --- a/drivers/media/dvb-frontends/stv090x.c +++ b/drivers/media/dvb-frontends/stv090x.c @@ -35,6 +35,9 @@ #include "stv090x.h" #include "stv090x_priv.h" +/* Max transfer size done by I2C transfer functions */ +#define MAX_XFER_SIZE 64 + static unsigned int verbose; module_param(verbose, int, 0644); @@ -722,9 +725,16 @@ static int stv090x_write_regs(struct stv090x_state *state, unsigned int reg, u8 { const struct stv090x_config *config = state->config; int ret; - u8 buf[2 + count]; + u8 buf[MAX_XFER_SIZE]; struct i2c_msg i2c_msg = { .addr = config->address, .flags = 0, .buf = buf, .len = 2 + count }; + if (2 + count > sizeof(buf)) { + printk(KERN_WARNING + "%s: i2c wr reg=%04x: len=%d is too big!\n", + KBUILD_MODNAME, reg, count); + return -EINVAL; + } + buf[0] = reg >> 8; buf[1] = reg & 0xff; memcpy(&buf[2], data, count); -- 1.8.3.1 -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 07/29] [media] platform drivers: Fix build on frv arch
On frv, the following errors happen: drivers/media/platform/soc_camera/rcar_vin.c: In function 'rcar_vin_setup': drivers/media/platform/soc_camera/rcar_vin.c:284:3: error: implicit declaration of function 'iowrite32' [-Werror=implicit-function-declaration] drivers/media/platform/soc_camera/rcar_vin.c: In function 'rcar_vin_request_capture_stop': drivers/media/platform/soc_camera/rcar_vin.c:353:2: error: implicit declaration of function 'ioread32' [-Werror=implicit-function-declaration] This is because this driver forgot to include linux/io.h. Signed-off-by: Mauro Carvalho Chehab --- drivers/media/platform/soc_camera/rcar_vin.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/media/platform/soc_camera/rcar_vin.c b/drivers/media/platform/soc_camera/rcar_vin.c index b21f777f55e7..6866bb4fbebc 100644 --- a/drivers/media/platform/soc_camera/rcar_vin.c +++ b/drivers/media/platform/soc_camera/rcar_vin.c @@ -16,6 +16,7 @@ #include #include +#include #include #include #include -- 1.8.3.1 -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 04/29] [media] cx18: struct i2c_client is too big for stack
drivers/media/pci/cx18/cx18-driver.c: In function 'cx18_read_eeprom': drivers/media/pci/cx18/cx18-driver.c:357:1: warning: the frame size of 1072 bytes is larger than 1024 bytes [-Wframe-larger-than=] That happens because the routine allocates 256 bytes for an eeprom buffer, plus the size of struct i2c_client, with is big. Change the logic to dynamically allocate/deallocate space for struct i2c_client, instead of using the stack. Signed-off-by: Mauro Carvalho Chehab --- drivers/media/pci/cx18/cx18-driver.c | 20 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/drivers/media/pci/cx18/cx18-driver.c b/drivers/media/pci/cx18/cx18-driver.c index ff7232023f56..87f5bcf29e90 100644 --- a/drivers/media/pci/cx18/cx18-driver.c +++ b/drivers/media/pci/cx18/cx18-driver.c @@ -324,23 +324,24 @@ static void cx18_eeprom_dump(struct cx18 *cx, unsigned char *eedata, int len) /* Hauppauge card? get values from tveeprom */ void cx18_read_eeprom(struct cx18 *cx, struct tveeprom *tv) { - struct i2c_client c; + struct i2c_client *c; u8 eedata[256]; - memset(&c, 0, sizeof(c)); - strlcpy(c.name, "cx18 tveeprom tmp", sizeof(c.name)); - c.adapter = &cx->i2c_adap[0]; - c.addr = 0xA0 >> 1; + c = kzalloc(sizeof(*c), GFP_ATOMIC); + + strlcpy(c->name, "cx18 tveeprom tmp", sizeof(c->name)); + c->adapter = &cx->i2c_adap[0]; + c->addr = 0xa0 >> 1; memset(tv, 0, sizeof(*tv)); - if (tveeprom_read(&c, eedata, sizeof(eedata))) - return; + if (tveeprom_read(c, eedata, sizeof(eedata))) + goto ret; switch (cx->card->type) { case CX18_CARD_HVR_1600_ESMT: case CX18_CARD_HVR_1600_SAMSUNG: case CX18_CARD_HVR_1600_S5H1411: - tveeprom_hauppauge_analog(&c, tv, eedata); + tveeprom_hauppauge_analog(c, tv, eedata); break; case CX18_CARD_YUAN_MPC718: case CX18_CARD_GOTVIEW_PCI_DVD3: @@ -354,6 +355,9 @@ void cx18_read_eeprom(struct cx18 *cx, struct tveeprom *tv) cx18_eeprom_dump(cx, eedata, sizeof(eedata)); break; } + +ret: + kfree(c); } static void cx18_process_eeprom(struct cx18 *cx) -- 1.8.3.1 -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 11/29] [media] s5h1420: Don't use dynamic static allocation
Dynamic static allocation is evil, as Kernel stack is too low, and compilation complains about it on some archs: drivers/media/dvb-frontends/s5h1420.c:851:1: warning: 's5h1420_tuner_i2c_tuner_xfer' uses dynamic stack allocation [enabled by default] Instead, let's enforce a limit for the buffer. In the specific case of this frontend, only ttpci uses it. The maximum number of messages there is two, on I2C read operations. As the logic can add an extra operation, change the size to 3. Signed-off-by: Mauro Carvalho Chehab --- drivers/media/dvb-frontends/s5h1420.c | 9 - 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/drivers/media/dvb-frontends/s5h1420.c b/drivers/media/dvb-frontends/s5h1420.c index e2fec9ebf947..97c400a4297f 100644 --- a/drivers/media/dvb-frontends/s5h1420.c +++ b/drivers/media/dvb-frontends/s5h1420.c @@ -836,9 +836,16 @@ static u32 s5h1420_tuner_i2c_func(struct i2c_adapter *adapter) static int s5h1420_tuner_i2c_tuner_xfer(struct i2c_adapter *i2c_adap, struct i2c_msg msg[], int num) { struct s5h1420_state *state = i2c_get_adapdata(i2c_adap); - struct i2c_msg m[1 + num]; + struct i2c_msg m[3]; u8 tx_open[2] = { CON_1, state->CON_1_val | 1 }; /* repeater stops once there was a stop condition */ + if (1 + num > ARRAY_SIZE(m)) { + printk(KERN_WARNING + "%s: i2c xfer: num=%d is too big!\n", + KBUILD_MODNAME, num); + return -EOPNOTSUPP; + } + memset(m, 0, sizeof(struct i2c_msg) * (1 + num)); m[0].addr = state->config->demod_address; -- 1.8.3.1 -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 13/29] [media] dvb-frontends: Don't use dynamic static allocation
Dynamic static allocation is evil, as Kernel stack is too low, and compilation complains about it on some archs: drivers/media/dvb-frontends/af9013.c:77:1: warning: 'af9013_wr_regs_i2c' uses dynamic stack allocation [enabled by default] drivers/media/dvb-frontends/af9033.c:188:1: warning: 'af9033_wr_reg_val_tab' uses dynamic stack allocation [enabled by default] drivers/media/dvb-frontends/af9033.c:68:1: warning: 'af9033_wr_regs' uses dynamic stack allocation [enabled by default] drivers/media/dvb-frontends/bcm3510.c:230:1: warning: 'bcm3510_do_hab_cmd' uses dynamic stack allocation [enabled by default] drivers/media/dvb-frontends/cxd2820r_core.c:84:1: warning: 'cxd2820r_rd_regs_i2c.isra.1' uses dynamic stack allocation [enabled by default] drivers/media/dvb-frontends/rtl2830.c:56:1: warning: 'rtl2830_wr' uses dynamic stack allocation [enabled by default] drivers/media/dvb-frontends/rtl2832.c:187:1: warning: 'rtl2832_wr' uses dynamic stack allocation [enabled by default] drivers/media/dvb-frontends/tda10071.c:52:1: warning: 'tda10071_wr_regs' uses dynamic stack allocation [enabled by default] drivers/media/dvb-frontends/tda10071.c:84:1: warning: 'tda10071_rd_regs' uses dynamic stack allocation [enabled by default] Instead, let's enforce a limit for the buffer. Considering that I2C transfers are generally limited, and that devices used on USB has a max data length of 64 bytes for the control URBs. So, it seem safe to use 64 bytes as the hard limit for all those devices. On most cases, the limit is a way lower than that, but this limit is small enough to not affect the Kernel stack, and it is a no brain limit, as using smaller ones would require to either carefully each driver or to take a look on each datasheet. Signed-off-by: Mauro Carvalho Chehab --- drivers/media/dvb-frontends/af9013.c| 12 +++- drivers/media/dvb-frontends/af9033.c| 21 +++-- drivers/media/dvb-frontends/cxd2820r_core.c | 21 +++-- drivers/media/dvb-frontends/rtl2830.c | 12 +++- drivers/media/dvb-frontends/rtl2832.c | 12 +++- drivers/media/dvb-frontends/tda10071.c | 21 +++-- 6 files changed, 90 insertions(+), 9 deletions(-) diff --git a/drivers/media/dvb-frontends/af9013.c b/drivers/media/dvb-frontends/af9013.c index a204f2828820..19ba66ad23fa 100644 --- a/drivers/media/dvb-frontends/af9013.c +++ b/drivers/media/dvb-frontends/af9013.c @@ -24,6 +24,9 @@ #include "af9013_priv.h" +/* Max transfer size done by I2C transfer functions */ +#define MAX_XFER_SIZE 64 + struct af9013_state { struct i2c_adapter *i2c; struct dvb_frontend fe; @@ -50,7 +53,7 @@ static int af9013_wr_regs_i2c(struct af9013_state *priv, u8 mbox, u16 reg, const u8 *val, int len) { int ret; - u8 buf[3+len]; + u8 buf[MAX_XFER_SIZE]; struct i2c_msg msg[1] = { { .addr = priv->config.i2c_addr, @@ -60,6 +63,13 @@ static int af9013_wr_regs_i2c(struct af9013_state *priv, u8 mbox, u16 reg, } }; + if (3 + len > sizeof(buf)) { + dev_warn(&priv->i2c->dev, +"%s: i2c wr reg=%04x: len=%d is too big!\n", +KBUILD_MODNAME, reg, len); + return -EINVAL; + } + buf[0] = (reg >> 8) & 0xff; buf[1] = (reg >> 0) & 0xff; buf[2] = mbox; diff --git a/drivers/media/dvb-frontends/af9033.c b/drivers/media/dvb-frontends/af9033.c index a777b4b944eb..11f1555e66dc 100644 --- a/drivers/media/dvb-frontends/af9033.c +++ b/drivers/media/dvb-frontends/af9033.c @@ -21,6 +21,9 @@ #include "af9033_priv.h" +/* Max transfer size done by I2C transfer functions */ +#define MAX_XFER_SIZE 64 + struct af9033_state { struct i2c_adapter *i2c; struct dvb_frontend fe; @@ -40,7 +43,7 @@ static int af9033_wr_regs(struct af9033_state *state, u32 reg, const u8 *val, int len) { int ret; - u8 buf[3 + len]; + u8 buf[MAX_XFER_SIZE]; struct i2c_msg msg[1] = { { .addr = state->cfg.i2c_addr, @@ -50,6 +53,13 @@ static int af9033_wr_regs(struct af9033_state *state, u32 reg, const u8 *val, } }; + if (3 + len > sizeof(buf)) { + dev_warn(&state->i2c->dev, +"%s: i2c wr reg=%04x: len=%d is too big!\n", +KBUILD_MODNAME, reg, len); + return -EINVAL; + } + buf[0] = (reg >> 16) & 0xff; buf[1] = (reg >> 8) & 0xff; buf[2] = (reg >> 0) & 0xff; @@ -161,7 +171,14 @@ static int af9033_wr_reg_val_tab(struct af9033_state *state, const struct reg_val *tab, int tab_len) { int ret, i, j; - u8 buf[tab_len]; + u8 buf[MAX_XFER_SIZE]; + + if (ta
[PATCH v3 19/29] [media] tuner-xc2028: Don't use dynamic static allocation
Dynamic static allocation is evil, as Kernel stack is too low, and compilation complains about it on some archs: drivers/media/tuners/tuner-xc2028.c:651:1: warning: 'load_firmware' uses dynamic stack allocation [enabled by default] Instead, let's enforce a limit for the buffer. In the specific case of this driver, the maximum limit is 80, used only on tm6000 driver. This limit is due to the size of the USB control URBs. Ok, it would be theoretically possible to use a bigger size on PCI devices, but the firmware load time is already good enough. Anyway, if some usage requires more, it is just a matter of also increasing the buffer size at load_firmware(). Signed-off-by: Mauro Carvalho Chehab --- drivers/media/tuners/tuner-xc2028.c | 8 +++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/drivers/media/tuners/tuner-xc2028.c b/drivers/media/tuners/tuner-xc2028.c index e287a7417319..4be5cf808a40 100644 --- a/drivers/media/tuners/tuner-xc2028.c +++ b/drivers/media/tuners/tuner-xc2028.c @@ -24,6 +24,9 @@ #include #include "dvb_frontend.h" +/* Max transfer size done by I2C transfer functions */ +#define MAX_XFER_SIZE 80 + /* Registers (Write-only) */ #define XREG_INIT 0x00 #define XREG_RF_FREQ 0x02 @@ -547,7 +550,10 @@ static int load_firmware(struct dvb_frontend *fe, unsigned int type, { struct xc2028_data *priv = fe->tuner_priv; intpos, rc; - unsigned char *p, *endp, buf[priv->ctrl.max_len]; + unsigned char *p, *endp, buf[MAX_XFER_SIZE]; + + if (priv->ctrl.max_len > sizeof(buf)) + priv->ctrl.max_len = sizeof(buf); tuner_dbg("%s called\n", __func__); -- 1.8.3.1 -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 01/29] [media] tda9887: remove an warning when compiling for alpha
There's no need to zero the buffer, as if the routine gets an error, rc will be different than one. That fixes the following warning: drivers/media/tuners/tda9887.c: In function 'tda9887_status': drivers/media/tuners/tda9887.c:539:2: warning: value computed is not used [-Wunused-value] While here, make fix the CodingStyle on this function. Signed-off-by: Mauro Carvalho Chehab --- drivers/media/tuners/tda9887.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/media/tuners/tda9887.c b/drivers/media/tuners/tda9887.c index 35c535ba..9823248d743f 100644 --- a/drivers/media/tuners/tda9887.c +++ b/drivers/media/tuners/tda9887.c @@ -536,8 +536,8 @@ static int tda9887_status(struct dvb_frontend *fe) unsigned char buf[1]; int rc; - memset(buf,0,sizeof(buf)); - if (1 != (rc = tuner_i2c_xfer_recv(&priv->i2c_props,buf,1))) + rc = tuner_i2c_xfer_recv(&priv->i2c_props, buf, 1); + if (rc != 1) tuner_info("i2c i/o error: rc == %d (should be 1)\n", rc); dump_read_message(fe, buf); return 0; -- 1.8.3.1 -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 10/29] [media] uvc/lirc_serial: Fix some warnings on parisc arch
On this arch, usec is not unsigned long. So, we need to typecast, in order to remove those warnings: drivers/media/usb/uvc/uvc_video.c: In function 'uvc_video_clock_update': drivers/media/usb/uvc/uvc_video.c:678:2: warning: format '%lu' expects argument of type 'long unsigned int', but argument 9 has type '__kernel_suseconds_t' [-Wformat] drivers/staging/media/lirc/lirc_serial.c: In function 'irq_handler': drivers/staging/media/lirc/lirc_serial.c:707:5: warning: format '%lx' expects argument of type 'long unsigned int', but argument 6 has type '__kernel_suseconds_t' [-Wformat] drivers/staging/media/lirc/lirc_serial.c:707:5: warning: format '%lx' expects argument of type 'long unsigned int', but argument 7 has type '__kernel_suseconds_t' [-Wformat] drivers/staging/media/lirc/lirc_serial.c:719:5: warning: format '%lx' expects argument of type 'long unsigned int', but argument 6 has type '__kernel_suseconds_t' [-Wformat] drivers/staging/media/lirc/lirc_serial.c:719:5: warning: format '%lx' expects argument of type 'long unsigned int', but argument 7 has type '__kernel_suseconds_t' [-Wformat] drivers/staging/media/lirc/lirc_serial.c:728:6: warning: format '%lx' expects argument of type 'long unsigned int', but argument 6 has type '__kernel_suseconds_t' [-Wformat] drivers/staging/media/lirc/lirc_serial.c:728:6: warning: format '%lx' expects argument of type 'long unsigned int', but argument 7 has type '__kernel_suseconds_t' [-Wformat] Signed-off-by: Mauro Carvalho Chehab Acked-by: Laurent Pinchart Signed-off-by: Mauro Carvalho Chehab --- drivers/media/usb/uvc/uvc_video.c| 3 ++- drivers/staging/media/lirc/lirc_serial.c | 9 ++--- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/drivers/media/usb/uvc/uvc_video.c b/drivers/media/usb/uvc/uvc_video.c index 3394c3432011..899cb6d1c4a4 100644 --- a/drivers/media/usb/uvc/uvc_video.c +++ b/drivers/media/usb/uvc/uvc_video.c @@ -680,7 +680,8 @@ void uvc_video_clock_update(struct uvc_streaming *stream, stream->dev->name, sof >> 16, div_u64(((u64)sof & 0x) * 100LLU, 65536), y, ts.tv_sec, ts.tv_nsec / NSEC_PER_USEC, - v4l2_buf->timestamp.tv_sec, v4l2_buf->timestamp.tv_usec, + v4l2_buf->timestamp.tv_sec, + (unsigned long)v4l2_buf->timestamp.tv_usec, x1, first->host_sof, first->dev_sof, x2, last->host_sof, last->dev_sof, y1, y2); diff --git a/drivers/staging/media/lirc/lirc_serial.c b/drivers/staging/media/lirc/lirc_serial.c index af08e677b60f..7b3be2346b4b 100644 --- a/drivers/staging/media/lirc/lirc_serial.c +++ b/drivers/staging/media/lirc/lirc_serial.c @@ -707,7 +707,8 @@ static irqreturn_t irq_handler(int i, void *blah) pr_warn("ignoring spike: %d %d %lx %lx %lx %lx\n", dcd, sense, tv.tv_sec, lasttv.tv_sec, - tv.tv_usec, lasttv.tv_usec); + (unsigned long)tv.tv_usec, + (unsigned long)lasttv.tv_usec); continue; } @@ -719,7 +720,8 @@ static irqreturn_t irq_handler(int i, void *blah) pr_warn("%d %d %lx %lx %lx %lx\n", dcd, sense, tv.tv_sec, lasttv.tv_sec, - tv.tv_usec, lasttv.tv_usec); + (unsigned long)tv.tv_usec, + (unsigned long)lasttv.tv_usec); data = PULSE_MASK; } else if (deltv > 15) { data = PULSE_MASK; /* really long time */ @@ -728,7 +730,8 @@ static irqreturn_t irq_handler(int i, void *blah) pr_warn("AI: %d %d %lx %lx %lx %lx\n", dcd, sense, tv.tv_sec, lasttv.tv_sec, - tv.tv_usec, lasttv.tv_usec); + (unsigned long)tv.tv_usec, + (unsigned long)lasttv.tv_usec); /* * detecting pulse while this * MUST be a space! -- 1.8.3.1 -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 12/29] [media] dvb-frontends: Don't use dynamic static allocation
Dynamic static allocation is evil, as Kernel stack is too low, and compilation complains about it on some archs: drivers/media/dvb-frontends/bcm3510.c:230:1: warning: 'bcm3510_do_hab_cmd' uses dynamic stack allocation [enabled by default] drivers/media/dvb-frontends/itd1000.c:69:1: warning: 'itd1000_write_regs.constprop.0' uses dynamic stack allocation [enabled by default] drivers/media/dvb-frontends/mt312.c:126:1: warning: 'mt312_write' uses dynamic stack allocation [enabled by default] drivers/media/dvb-frontends/nxt200x.c:111:1: warning: 'nxt200x_writebytes' uses dynamic stack allocation [enabled by default] drivers/media/dvb-frontends/stb6100.c:216:1: warning: 'stb6100_write_reg_range.constprop.3' uses dynamic stack allocation [enabled by default] drivers/media/dvb-frontends/stv6110.c:98:1: warning: 'stv6110_write_regs' uses dynamic stack allocation [enabled by default] drivers/media/dvb-frontends/stv6110x.c:85:1: warning: 'stv6110x_write_regs' uses dynamic stack allocation [enabled by default] drivers/media/dvb-frontends/tda18271c2dd.c:147:1: warning: 'WriteRegs' uses dynamic stack allocation [enabled by default] drivers/media/dvb-frontends/zl10039.c:119:1: warning: 'zl10039_write' uses dynamic stack allocation [enabled by default] Instead, let's enforce a limit for the buffer. Considering that I2C transfers are generally limited, and that devices used on USB has a max data length of 64 bytes for the control URBs. So, it seem safe to use 64 bytes as the hard limit for all those devices. On most cases, the limit is a way lower than that, but this limit is small enough to not affect the Kernel stack, and it is a no brain limit, as using smaller ones would require to either carefully each driver or to take a look on each datasheet. Signed-off-by: Mauro Carvalho Chehab --- drivers/media/dvb-frontends/bcm3510.c | 15 ++- drivers/media/dvb-frontends/itd1000.c | 13 - drivers/media/dvb-frontends/mt312.c| 10 +- drivers/media/dvb-frontends/nxt200x.c | 11 ++- drivers/media/dvb-frontends/stb6100.c | 11 ++- drivers/media/dvb-frontends/stv6110.c | 12 +++- drivers/media/dvb-frontends/stv6110x.c | 13 - drivers/media/dvb-frontends/tda18271c2dd.c | 14 -- drivers/media/dvb-frontends/zl10039.c | 12 +++- 9 files changed, 101 insertions(+), 10 deletions(-) diff --git a/drivers/media/dvb-frontends/bcm3510.c b/drivers/media/dvb-frontends/bcm3510.c index 1b77909c0c71..39a29dd29519 100644 --- a/drivers/media/dvb-frontends/bcm3510.c +++ b/drivers/media/dvb-frontends/bcm3510.c @@ -44,6 +44,9 @@ #include "bcm3510.h" #include "bcm3510_priv.h" +/* Max transfer size done by bcm3510_do_hab_cmd() function */ +#define MAX_XFER_SIZE 128 + struct bcm3510_state { struct i2c_adapter* i2c; @@ -201,9 +204,19 @@ static int bcm3510_hab_send_request(struct bcm3510_state *st, u8 *buf, int len) static int bcm3510_do_hab_cmd(struct bcm3510_state *st, u8 cmd, u8 msgid, u8 *obuf, u8 olen, u8 *ibuf, u8 ilen) { - u8 ob[olen+2],ib[ilen+2]; + u8 ob[MAX_XFER_SIZE], ib[MAX_XFER_SIZE]; int ret = 0; + if (ilen + 2 > sizeof(ib)) { + deb_hab("do_hab_cmd: ilen=%d is too big!\n", ilen); + return -EINVAL; + } + + if (olen + 2 > sizeof(ob)) { + deb_hab("do_hab_cmd: olen=%d is too big!\n", olen); + return -EINVAL; + } + ob[0] = cmd; ob[1] = msgid; memcpy(&ob[2],obuf,olen); diff --git a/drivers/media/dvb-frontends/itd1000.c b/drivers/media/dvb-frontends/itd1000.c index c1c3400b2173..cadcae4cff89 100644 --- a/drivers/media/dvb-frontends/itd1000.c +++ b/drivers/media/dvb-frontends/itd1000.c @@ -31,6 +31,9 @@ #include "itd1000.h" #include "itd1000_priv.h" +/* Max transfer size done by I2C transfer functions */ +#define MAX_XFER_SIZE 64 + static int debug; module_param(debug, int, 0644); MODULE_PARM_DESC(debug, "Turn on/off debugging (default:off)."); @@ -52,10 +55,18 @@ MODULE_PARM_DESC(debug, "Turn on/off debugging (default:off)."); /* don't write more than one byte with flexcop behind */ static int itd1000_write_regs(struct itd1000_state *state, u8 reg, u8 v[], u8 len) { - u8 buf[1+len]; + u8 buf[MAX_XFER_SIZE]; struct i2c_msg msg = { .addr = state->cfg->i2c_address, .flags = 0, .buf = buf, .len = len+1 }; + + if (1 + len > sizeof(buf)) { + printk(KERN_WARNING + "itd1000: i2c wr reg=%04x: len=%d is too big!\n", + reg, len); + return -EINVAL; + } + buf[0] = reg; memcpy(&buf[1], v, len); diff --git a/drivers/media/dvb-frontends/mt312.c b/drivers/media/dvb-frontends/mt312.c index ec388c1d6913..a74ac0ddb833 100644 --- a/drivers/media/dvb-fron
[PATCH v3 03/29] [media] zoran: don't build it on alpha
This driver uses virt_to_bus() with is deprecated on Alpha: drivers/media/pci/zoran/zoran_device.c: In function 'zr36057_set_vfe': drivers/media/pci/zoran/zoran_device.c:451:3: warning: 'virt_to_bus' is deprecated (declared at /devel/v4l/ktest-build/arch/alpha/include/asm/io.h:114) [-Wdeprecated-declarations] drivers/media/pci/zoran/zoran_device.c:453:3: warning: 'virt_to_bus' is deprecated (declared at /devel/v4l/ktest-build/arch/alpha/include/asm/io.h:114) [-Wdeprecated-declarations] drivers/media/pci/zoran/zoran_device.c: In function 'zr36057_set_jpg': drivers/media/pci/zoran/zoran_device.c:796:2: warning: 'virt_to_bus' is deprecated (declared at /devel/v4l/ktest-build/arch/alpha/include/asm/io.h:114) [-Wdeprecated-declarations] drivers/media/pci/zoran/zoran_driver.c: In function 'v4l_fbuffer_alloc': drivers/media/pci/zoran/zoran_driver.c:241:3: warning: 'virt_to_bus' is deprecated (declared at /devel/v4l/ktest-build/arch/alpha/include/asm/io.h:114) [-Wdeprecated-declarations] drivers/media/pci/zoran/zoran_driver.c:245:3: warning: 'virt_to_bus' is deprecated (declared at /devel/v4l/ktest-build/arch/alpha/include/asm/io.h:114) [-Wdeprecated-declarations] drivers/media/pci/zoran/zoran_driver.c: In function 'jpg_fbuffer_alloc': drivers/media/pci/zoran/zoran_driver.c:334:3: warning: 'virt_to_bus' is deprecated (declared at /devel/v4l/ktest-build/arch/alpha/include/asm/io.h:114) [-Wdeprecated-declarations] drivers/media/pci/zoran/zoran_driver.c:347:5: warning: 'virt_to_bus' is deprecated (declared at /devel/v4l/ktest-build/arch/alpha/include/asm/io.h:114) [-Wdeprecated-declarations] drivers/media/pci/zoran/zoran_driver.c:366:6: warning: 'virt_to_bus' is deprecated (declared at /devel/v4l/ktest-build/arch/alpha/include/asm/io.h:114) [-Wdeprecated-declarations] As we're not even sure if it works on Alpha, better to just disable its compilation there. Signed-off-by: Mauro Carvalho Chehab --- drivers/media/pci/zoran/Kconfig | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/media/pci/zoran/Kconfig b/drivers/media/pci/zoran/Kconfig index 26ca8702e33f..39ec35bd21a5 100644 --- a/drivers/media/pci/zoran/Kconfig +++ b/drivers/media/pci/zoran/Kconfig @@ -1,6 +1,7 @@ config VIDEO_ZORAN tristate "Zoran ZR36057/36067 Video For Linux" depends on PCI && I2C_ALGOBIT && VIDEO_V4L2 && VIRT_TO_BUS + depends on !ALPHA help Say Y for support for MJPEG capture cards based on the Zoran 36057/36067 PCI controller chipset. This includes the Iomega -- 1.8.3.1 -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 00/29] Fix errors/warnings with allmodconfig/allyesconfig on non-x86 archs
To be sure that we're not introducing compilation regressions on media, I'm now using ktest to check for errors/warnings. My current setup is cross-building on several architectures: alpha, arm, avr32, cris (64), frv, i386, ia64, m32r, m68k, mips, openrisc, parisc, s390, sh, sparc, sparc64, uml, x86_64 I tried to enable a few other archs: blackfin, cris (32), powerpc (32, 64), tile, xtensa but they fail to compile with allyesconfig due to non-media related issues. I'm still unsure about how often I'll be doing it, I intend to run it at least by the end of the subsystem merge window (by -rc6 or -rc7), and fix the issues found there. V3: contains fixes for the feedbacks received: - I2C client driver now returns -EINVAL; - I2C adapter drivers now returns -EOPNOSUPP; - a macro was added for the buffers; - check for failure on allocation at v4l2-async was added; - Used iguanair code from Sean, instead of my code; - Most buffer sizes changed to 64 bytes, to match max URB control size. Mauro Carvalho Chehab (28): [media] tda9887: remove an warning when compiling for alpha [media] radio-shark: remove a warning when CONFIG_PM is not defined [media] zoran: don't build it on alpha [media] cx18: struct i2c_client is too big for stack [media] tef6862: fix warning on avr32 arch [media] platform drivers: Fix build on frv arch [media] radio-si470x-i2c: fix a warning on ia64 [media] rc: Fir warnings on m68k arch [media] uvc/lirc_serial: Fix some warnings on parisc arch [media] s5h1420: Don't use dynamic static allocation [media] dvb-frontends: Don't use dynamic static allocation [media] dvb-frontends: Don't use dynamic static allocation [media] stb0899_drv: Don't use dynamic static allocation [media] stv0367: Don't use dynamic static allocation [media] stv090x: Don't use dynamic static allocation [media] av7110_hw: Don't use dynamic static allocation [media] tuners: Don't use dynamic static allocation [media] tuner-xc2028: Don't use dynamic static allocation [media] cimax2: Don't use dynamic static allocation [media] v4l2-async: Don't use dynamic static allocation [media] cxusb: Don't use dynamic static allocation [media] dibusb-common: Don't use dynamic static allocation [media] dw2102: Don't use dynamic static allocation [media] af9015: Don't use dynamic static allocation [media] af9035: Don't use dynamic static allocation [media] mxl111sf: Don't use dynamic static allocation [media] lirc_zilog: Don't use dynamic static allocation [media] cx18: disable compilation on frv arch Sean Young (1): [media] iguanair: simplify calculation of carrier delay cycles drivers/media/dvb-frontends/af9013.c | 12 +++- drivers/media/dvb-frontends/af9033.c | 21 ++- drivers/media/dvb-frontends/bcm3510.c | 15 - drivers/media/dvb-frontends/cxd2820r_core.c | 21 ++- drivers/media/dvb-frontends/itd1000.c | 13 +++- drivers/media/dvb-frontends/mt312.c | 10 ++- drivers/media/dvb-frontends/nxt200x.c | 11 +++- drivers/media/dvb-frontends/rtl2830.c | 12 +++- drivers/media/dvb-frontends/rtl2832.c | 12 +++- drivers/media/dvb-frontends/s5h1420.c | 9 ++- drivers/media/dvb-frontends/stb0899_drv.c | 12 +++- drivers/media/dvb-frontends/stb6100.c | 11 +++- drivers/media/dvb-frontends/stv0367.c | 13 +++- drivers/media/dvb-frontends/stv090x.c | 12 +++- drivers/media/dvb-frontends/stv6110.c | 12 +++- drivers/media/dvb-frontends/stv6110x.c| 13 +++- drivers/media/dvb-frontends/tda10071.c| 21 ++- drivers/media/dvb-frontends/tda18271c2dd.c| 14 - drivers/media/dvb-frontends/zl10039.c | 12 +++- drivers/media/pci/cx18/Kconfig| 1 + drivers/media/pci/cx18/cx18-driver.c | 20 +++--- drivers/media/pci/cx23885/cimax2.c| 13 +++- drivers/media/pci/ttpci/av7110_hw.c | 19 +- drivers/media/pci/zoran/Kconfig | 1 + drivers/media/platform/soc_camera/rcar_vin.c | 1 + drivers/media/radio/radio-shark.c | 2 + drivers/media/radio/radio-shark2.c| 2 + drivers/media/radio/si470x/radio-si470x-i2c.c | 4 +- drivers/media/radio/tef6862.c | 20 +++--- drivers/media/rc/fintek-cir.h | 4 +- drivers/media/rc/iguanair.c | 22 ++- drivers/media/rc/nuvoton-cir.h| 4 +- drivers/media/tuners/e4000.c | 21 ++- drivers/media/tuners/fc2580.c | 21 ++- drivers/media/tuners/tda18212.c | 21 ++- drivers/media/tuners/tda18218.c | 21 ++- drivers/media/tuners/tda9887.c| 4 +- drivers/media/tuners/tuner-xc2028.c | 8 ++- drivers/media/usb/dvb-usb-v2/af9015.c | 3 +- drivers/media/usb/dvb-usb-v2/af9035.c
[PATCH v3 28/29] [media] lirc_zilog: Don't use dynamic static allocation
Dynamic static allocation is evil, as Kernel stack is too low, and ompilation complains about it on some archs: drivers/staging/media/lirc/lirc_zilog.c:967:1: warning: 'read' uses dynamic stack allocation [enabled by default] Instead, let's enforce a limit for the buffer to be 64. That should be more than enough. Signed-off-by: Mauro Carvalho Chehab --- drivers/staging/media/lirc/lirc_zilog.c | 12 +++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/drivers/staging/media/lirc/lirc_zilog.c b/drivers/staging/media/lirc/lirc_zilog.c index 11d5338b4f2f..0feeaadf29dc 100644 --- a/drivers/staging/media/lirc/lirc_zilog.c +++ b/drivers/staging/media/lirc/lirc_zilog.c @@ -61,6 +61,9 @@ #include #include +/* Max transfer size done by I2C transfer functions */ +#define MAX_XFER_SIZE 64 + struct IR; struct IR_rx { @@ -941,7 +944,14 @@ static ssize_t read(struct file *filep, char *outbuf, size_t n, loff_t *ppos) schedule(); set_current_state(TASK_INTERRUPTIBLE); } else { - unsigned char buf[rbuf->chunk_size]; + unsigned char buf[MAX_XFER_SIZE]; + + if (rbuf->chunk_size > sizeof(buf)) { + zilog_error("chunk_size is too big (%d)!\n", + rbuf->chunk_size); + ret = -EINVAL; + break; + } m = lirc_buffer_read(rbuf, buf); if (m == rbuf->chunk_size) { ret = copy_to_user((void *)outbuf+written, buf, -- 1.8.3.1 -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 24/29] [media] dw2102: Don't use dynamic static allocation
Dynamic static allocation is evil, as Kernel stack is too low, and compilation complains about it on some archs: drivers/media/usb/dvb-usb/dw2102.c:368:1: warning: 'dw2102_earda_i2c_transfer' uses dynamic stack allocation [enabled by default] drivers/media/usb/dvb-usb/dw2102.c:449:1: warning: 'dw2104_i2c_transfer' uses dynamic stack allocation [enabled by default] drivers/media/usb/dvb-usb/dw2102.c:512:1: warning: 'dw3101_i2c_transfer' uses dynamic stack allocation [enabled by default] drivers/media/usb/dvb-usb/dw2102.c:621:1: warning: 's6x0_i2c_transfer' uses dynamic stack allocation [enabled by default] Instead, let's enforce a limit for the buffer to be the max size of a control URB payload data (64 bytes). Signed-off-by: Mauro Carvalho Chehab --- drivers/media/usb/dvb-usb/dw2102.c | 90 +- 1 file changed, 80 insertions(+), 10 deletions(-) diff --git a/drivers/media/usb/dvb-usb/dw2102.c b/drivers/media/usb/dvb-usb/dw2102.c index 6136a2c7dbfd..c1a63b2a6baa 100644 --- a/drivers/media/usb/dvb-usb/dw2102.c +++ b/drivers/media/usb/dvb-usb/dw2102.c @@ -30,6 +30,9 @@ #include "stb6100_proc.h" #include "m88rs2000.h" +/* Max transfer size done by I2C transfer functions */ +#define MAX_XFER_SIZE 64 + #ifndef USB_PID_DW2102 #define USB_PID_DW2102 0x2102 #endif @@ -308,7 +311,14 @@ static int dw2102_earda_i2c_transfer(struct i2c_adapter *adap, struct i2c_msg ms case 2: { /* read */ /* first write first register number */ - u8 ibuf[msg[1].len + 2], obuf[3]; + u8 ibuf[MAX_XFER_SIZE], obuf[3]; + + if (2 + msg[1].len > sizeof(ibuf)) { + warn("i2c rd: len=%d is too big!\n", +msg[1].len); + return -EOPNOTSUPP; + } + obuf[0] = msg[0].addr << 1; obuf[1] = msg[0].len; obuf[2] = msg[0].buf[0]; @@ -325,7 +335,14 @@ static int dw2102_earda_i2c_transfer(struct i2c_adapter *adap, struct i2c_msg ms switch (msg[0].addr) { case 0x68: { /* write to register */ - u8 obuf[msg[0].len + 2]; + u8 obuf[MAX_XFER_SIZE]; + + if (2 + msg[0].len > sizeof(obuf)) { + warn("i2c wr: len=%d is too big!\n", +msg[1].len); + return -EOPNOTSUPP; + } + obuf[0] = msg[0].addr << 1; obuf[1] = msg[0].len; memcpy(obuf + 2, msg[0].buf, msg[0].len); @@ -335,7 +352,14 @@ static int dw2102_earda_i2c_transfer(struct i2c_adapter *adap, struct i2c_msg ms } case 0x61: { /* write to tuner */ - u8 obuf[msg[0].len + 2]; + u8 obuf[MAX_XFER_SIZE]; + + if (2 + msg[0].len > sizeof(obuf)) { + warn("i2c wr: len=%d is too big!\n", +msg[1].len); + return -EOPNOTSUPP; + } + obuf[0] = msg[0].addr << 1; obuf[1] = msg[0].len; memcpy(obuf + 2, msg[0].buf, msg[0].len); @@ -401,7 +425,14 @@ static int dw2104_i2c_transfer(struct i2c_adapter *adap, struct i2c_msg msg[], i default: { if (msg[j].flags == I2C_M_RD) { /* read registers */ - u8 ibuf[msg[j].len + 2]; + u8 ibuf[MAX_XFER_SIZE]; + + if (2 + msg[j].len > sizeof(ibuf)) { + warn("i2c rd: len=%d is too big!\n", +msg[j].len); + return -EOPNOTSUPP; + } + dw210x_op_rw(d->udev, 0xc3, (msg[j].addr << 1) + 1, 0, ibuf, msg[j].len + 2, @@ -430,7 +461,14 @@ static int dw2104_i2c_transfer(struct i2c_adapter *adap, struct i2c_msg msg[], i } while (len > 0); } else { /* write registers */ - u8 obuf[msg[j].len + 2]; + u8 obuf[MAX_XFER_SIZE]; + + if (2 + msg[j].len > sizeof(obuf)) { + warn("i2c wr: len=%d is too big!\n", +msg[j].len); + return -EOPNOTSUPP; + } +
[PATCH v3 09/29] [media] rc: Fir warnings on m68k arch
Fix the following warnings: drivers/media/rc/fintek-cir.c: In function 'fintek_cr_write': drivers/media/rc/fintek-cir.c:45:2: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast] drivers/media/rc/fintek-cir.c:46:2: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast] drivers/media/rc/fintek-cir.c: In function 'fintek_cr_read': drivers/media/rc/fintek-cir.c:54:2: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast] drivers/media/rc/fintek-cir.c:55:8: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast] drivers/media/rc/fintek-cir.c: In function 'fintek_config_mode_enable': drivers/media/rc/fintek-cir.c:80:2: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast] drivers/media/rc/fintek-cir.c:81:2: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast] drivers/media/rc/fintek-cir.c: In function 'fintek_config_mode_disable': drivers/media/rc/fintek-cir.c:87:2: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast] drivers/media/rc/nuvoton-cir.c: In function 'nvt_cr_write': drivers/media/rc/nuvoton-cir.c:45:2: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast] drivers/media/rc/nuvoton-cir.c:46:2: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast] drivers/media/rc/nuvoton-cir.c: In function 'nvt_cr_read': drivers/media/rc/nuvoton-cir.c:52:2: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast] drivers/media/rc/nuvoton-cir.c:53:9: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast] drivers/media/rc/nuvoton-cir.c: In function 'nvt_efm_enable': drivers/media/rc/nuvoton-cir.c:74:2: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast] drivers/media/rc/nuvoton-cir.c:75:2: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast] drivers/media/rc/nuvoton-cir.c: In function 'nvt_efm_disable': drivers/media/rc/nuvoton-cir.c:81:2: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast] drivers/media/rc/nuvoton-cir.c: In function 'nvt_select_logical_dev': drivers/media/rc/nuvoton-cir.c:91:2: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast] drivers/media/rc/nuvoton-cir.c:92:2: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast] Those are caused because the I/O port is u32, instead of u8. Signed-off-by: Mauro Carvalho Chehab --- drivers/media/rc/fintek-cir.h | 4 ++-- drivers/media/rc/nuvoton-cir.h | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/media/rc/fintek-cir.h b/drivers/media/rc/fintek-cir.h index 82516a1d39b0..b698f3d2ced9 100644 --- a/drivers/media/rc/fintek-cir.h +++ b/drivers/media/rc/fintek-cir.h @@ -76,8 +76,8 @@ struct fintek_dev { } tx; /* Config register index/data port pair */ - u8 cr_ip; - u8 cr_dp; + u32 cr_ip; + u32 cr_dp; /* hardware I/O settings */ unsigned long cir_addr; diff --git a/drivers/media/rc/nuvoton-cir.h b/drivers/media/rc/nuvoton-cir.h index 7c3674ff5ea2..07e83108df0f 100644 --- a/drivers/media/rc/nuvoton-cir.h +++ b/drivers/media/rc/nuvoton-cir.h @@ -84,8 +84,8 @@ struct nvt_dev { } tx; /* EFER Config register index/data pair */ - u8 cr_efir; - u8 cr_efdr; + u32 cr_efir; + u32 cr_efdr; /* hardware I/O settings */ unsigned long cir_addr; -- 1.8.3.1 -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 05/29] [media] tef6862: fix warning on avr32 arch
On avr32 arch, we get those warnings: drivers/media/radio/tef6862.c:59:1: warning: "MODE_SHIFT" redefined In file included from /devel/v4l/ktest-build/arch/avr32/include/asm/ptrace.h:11, arch/avr32/include/uapi/asm/ptrace.h:41:1: warning: this is the location of the previous definition Prefix MSA_ to the MSA register bitmap macros, to avoid reusing the same symbol. Signed-off-by: Mauro Carvalho Chehab --- drivers/media/radio/tef6862.c | 20 ++-- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/drivers/media/radio/tef6862.c b/drivers/media/radio/tef6862.c index 06ac69245ca1..69e3245a58a0 100644 --- a/drivers/media/radio/tef6862.c +++ b/drivers/media/radio/tef6862.c @@ -48,15 +48,15 @@ #define WM_SUB_TEST0xF /* Different modes of the MSA register */ -#define MODE_BUFFER0x0 -#define MODE_PRESET0x1 -#define MODE_SEARCH0x2 -#define MODE_AF_UPDATE 0x3 -#define MODE_JUMP 0x4 -#define MODE_CHECK 0x5 -#define MODE_LOAD 0x6 -#define MODE_END 0x7 -#define MODE_SHIFT 5 +#define MSA_MODE_BUFFER0x0 +#define MSA_MODE_PRESET0x1 +#define MSA_MODE_SEARCH0x2 +#define MSA_MODE_AF_UPDATE 0x3 +#define MSA_MODE_JUMP 0x4 +#define MSA_MODE_CHECK 0x5 +#define MSA_MODE_LOAD 0x6 +#define MSA_MODE_END 0x7 +#define MSA_MODE_SHIFT 5 struct tef6862_state { struct v4l2_subdev sd; @@ -114,7 +114,7 @@ static int tef6862_s_frequency(struct v4l2_subdev *sd, const struct v4l2_frequen clamp(freq, TEF6862_LO_FREQ, TEF6862_HI_FREQ); pll = 1964 + ((freq - TEF6862_LO_FREQ) * 20) / FREQ_MUL; - i2cmsg[0] = (MODE_PRESET << MODE_SHIFT) | WM_SUB_PLLM; + i2cmsg[0] = (MSA_MODE_PRESET << MSA_MODE_SHIFT) | WM_SUB_PLLM; i2cmsg[1] = (pll >> 8) & 0xff; i2cmsg[2] = pll & 0xff; -- 1.8.3.1 -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 22/29] [media] cxusb: Don't use dynamic static allocation
Dynamic static allocation is evil, as Kernel stack is too low, and compilation complains about it on some archs: drivers/media/usb/dvb-usb/cxusb.c:209:1: warning: 'cxusb_i2c_xfer' uses dynamic stack allocation [enabled by default] drivers/media/usb/dvb-usb/cxusb.c:69:1: warning: 'cxusb_ctrl_msg' uses dynamic stack allocation [enabled by default] Instead, let's enforce a limit for the buffer to be the max size of a control URB payload data (64 bytes). Signed-off-by: Mauro Carvalho Chehab --- drivers/media/usb/dvb-usb/cxusb.c | 41 +++ 1 file changed, 37 insertions(+), 4 deletions(-) diff --git a/drivers/media/usb/dvb-usb/cxusb.c b/drivers/media/usb/dvb-usb/cxusb.c index 3940bb0f9ef6..20e345d9fe8f 100644 --- a/drivers/media/usb/dvb-usb/cxusb.c +++ b/drivers/media/usb/dvb-usb/cxusb.c @@ -43,6 +43,9 @@ #include "lgs8gxx.h" #include "atbm8830.h" +/* Max transfer size done by I2C transfer functions */ +#define MAX_XFER_SIZE 64 + /* debug */ static int dvb_usb_cxusb_debug; module_param_named(debug, dvb_usb_cxusb_debug, int, 0644); @@ -57,7 +60,14 @@ static int cxusb_ctrl_msg(struct dvb_usb_device *d, u8 cmd, u8 *wbuf, int wlen, u8 *rbuf, int rlen) { int wo = (rbuf == NULL || rlen == 0); /* write-only */ - u8 sndbuf[1+wlen]; + u8 sndbuf[MAX_XFER_SIZE]; + + if (1 + wlen > sizeof(sndbuf)) { + warn("i2c wr: len=%d is too big!\n", +wlen); + return -EOPNOTSUPP; + } + memset(sndbuf, 0, 1+wlen); sndbuf[0] = cmd; @@ -158,7 +168,13 @@ static int cxusb_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg msg[], if (msg[i].flags & I2C_M_RD) { /* read only */ - u8 obuf[3], ibuf[1+msg[i].len]; + u8 obuf[3], ibuf[MAX_XFER_SIZE]; + + if (1 + msg[i].len > sizeof(ibuf)) { + warn("i2c rd: len=%d is too big!\n", +msg[i].len); + return -EOPNOTSUPP; + } obuf[0] = 0; obuf[1] = msg[i].len; obuf[2] = msg[i].addr; @@ -172,7 +188,18 @@ static int cxusb_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg msg[], } else if (i+1 < num && (msg[i+1].flags & I2C_M_RD) && msg[i].addr == msg[i+1].addr) { /* write to then read from same address */ - u8 obuf[3+msg[i].len], ibuf[1+msg[i+1].len]; + u8 obuf[MAX_XFER_SIZE], ibuf[MAX_XFER_SIZE]; + + if (3 + msg[i].len > sizeof(obuf)) { + warn("i2c wr: len=%d is too big!\n", +msg[i].len); + return -EOPNOTSUPP; + } + if (1 + msg[i + 1].len > sizeof(ibuf)) { + warn("i2c rd: len=%d is too big!\n", +msg[i + 1].len); + return -EOPNOTSUPP; + } obuf[0] = msg[i].len; obuf[1] = msg[i+1].len; obuf[2] = msg[i].addr; @@ -191,7 +218,13 @@ static int cxusb_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg msg[], i++; } else { /* write only */ - u8 obuf[2+msg[i].len], ibuf; + u8 obuf[MAX_XFER_SIZE], ibuf; + + if (2 + msg[i].len > sizeof(obuf)) { + warn("i2c wr: len=%d is too big!\n", +msg[i].len); + return -EOPNOTSUPP; + } obuf[0] = msg[i].addr; obuf[1] = msg[i].len; memcpy(&obuf[2], msg[i].buf, msg[i].len); -- 1.8.3.1 -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 27/29] [media] mxl111sf: Don't use dynamic static allocation
Dynamic static allocation is evil, as Kernel stack is too low, and compilation complains about it on some archs: drivers/media/usb/dvb-usb-v2/mxl111sf.c:74:1: warning: 'mxl111sf_ctrl_msg' uses dynamic stack allocation [enabled by default] Instead, let's enforce a limit for the buffer to be the max size of a control URB payload data (64 bytes). Signed-off-by: Mauro Carvalho Chehab --- drivers/media/usb/dvb-usb-v2/mxl111sf.c | 10 +- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/drivers/media/usb/dvb-usb-v2/mxl111sf.c b/drivers/media/usb/dvb-usb-v2/mxl111sf.c index e97964ef7f56..2627553f7de1 100644 --- a/drivers/media/usb/dvb-usb-v2/mxl111sf.c +++ b/drivers/media/usb/dvb-usb-v2/mxl111sf.c @@ -23,6 +23,9 @@ #include "lgdt3305.h" #include "lg2160.h" +/* Max transfer size done by I2C transfer functions */ +#define MAX_XFER_SIZE 64 + int dvb_usb_mxl111sf_debug; module_param_named(debug, dvb_usb_mxl111sf_debug, int, 0644); MODULE_PARM_DESC(debug, "set debugging level " @@ -57,7 +60,12 @@ int mxl111sf_ctrl_msg(struct dvb_usb_device *d, { int wo = (rbuf == NULL || rlen == 0); /* write-only */ int ret; - u8 sndbuf[1+wlen]; + u8 sndbuf[MAX_XFER_SIZE]; + + if (1 + wlen > sizeof(sndbuf)) { + pr_warn("%s: len=%d is too big!\n", __func__, wlen); + return -EOPNOTSUPP; + } pr_debug("%s(wlen = %d, rlen = %d)\n", __func__, wlen, rlen); -- 1.8.3.1 -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 20/29] [media] cimax2: Don't use dynamic static allocation
Dynamic static allocation is evil, as Kernel stack is too low, and compilation complains about it on some archs: drivers/media/pci/cx23885/cimax2.c:149:1: warning: 'netup_write_i2c' uses dynamic stack allocation [enabled by default] Instead, let's enforce a limit for the buffer. Considering that I2C transfers are generally limited, and that devices used on USB has a max data length of 64 bytes for the control URBs. So, it seem safe to use 64 bytes as the hard limit for all those devices. On most cases, the limit is a way lower than that, but this limit is small enough to not affect the Kernel stack, and it is a no brain limit, as using smaller ones would require to either carefully each driver or to take a look on each datasheet. Signed-off-by: Mauro Carvalho Chehab --- drivers/media/pci/cx23885/cimax2.c | 13 - 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/drivers/media/pci/cx23885/cimax2.c b/drivers/media/pci/cx23885/cimax2.c index 7344849183a7..16fa7ea4d4aa 100644 --- a/drivers/media/pci/cx23885/cimax2.c +++ b/drivers/media/pci/cx23885/cimax2.c @@ -26,6 +26,10 @@ #include "cx23885.h" #include "cimax2.h" #include "dvb_ca_en50221.h" + +/* Max transfer size done by I2C transfer functions */ +#define MAX_XFER_SIZE 64 + / Bit definitions for MC417_RWD and MC417_OEN registers *** bits 31-16 +---+ @@ -125,7 +129,7 @@ static int netup_write_i2c(struct i2c_adapter *i2c_adap, u8 addr, u8 reg, u8 *buf, int len) { int ret; - u8 buffer[len + 1]; + u8 buffer[MAX_XFER_SIZE]; struct i2c_msg msg = { .addr = addr, @@ -134,6 +138,13 @@ static int netup_write_i2c(struct i2c_adapter *i2c_adap, u8 addr, u8 reg, .len= len + 1 }; + if (1 + len > sizeof(buffer)) { + printk(KERN_WARNING + "%s: i2c wr reg=%04x: len=%d is too big!\n", + KBUILD_MODNAME, reg, len); + return -EINVAL; + } + buffer[0] = reg; memcpy(&buffer[1], buf, len); -- 1.8.3.1 -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 15/29] [media] stv0367: Don't use dynamic static allocation
Dynamic static allocation is evil, as Kernel stack is too low, and compilation complains about it on some archs: drivers/media/dvb-frontends/stv0367.c:791:1: warning: 'stv0367_writeregs.constprop.4' uses dynamic stack allocation [enabled by default] Instead, let's enforce a limit for the buffer. Considering that I2C transfers are generally limited, and that devices used on USB has a max data length of 64 bytes for the control URBs. So, it seem safe to use 64 bytes as the hard limit for all those devices. On most cases, the limit is a way lower than that, but this limit is small enough to not affect the Kernel stack, and it is a no brain limit, as using smaller ones would require to either carefully each driver or to take a look on each datasheet. Signed-off-by: Mauro Carvalho Chehab --- drivers/media/dvb-frontends/stv0367.c | 13 - 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/drivers/media/dvb-frontends/stv0367.c b/drivers/media/dvb-frontends/stv0367.c index 7b6dba3ce55e..458772739423 100644 --- a/drivers/media/dvb-frontends/stv0367.c +++ b/drivers/media/dvb-frontends/stv0367.c @@ -33,6 +33,9 @@ #include "stv0367_regs.h" #include "stv0367_priv.h" +/* Max transfer size done by I2C transfer functions */ +#define MAX_XFER_SIZE 64 + static int stvdebug; module_param_named(debug, stvdebug, int, 0644); @@ -767,7 +770,7 @@ static struct st_register def0367cab[STV0367CAB_NBREGS] = { static int stv0367_writeregs(struct stv0367_state *state, u16 reg, u8 *data, int len) { - u8 buf[len + 2]; + u8 buf[MAX_XFER_SIZE]; struct i2c_msg msg = { .addr = state->config->demod_address, .flags = 0, @@ -776,6 +779,14 @@ int stv0367_writeregs(struct stv0367_state *state, u16 reg, u8 *data, int len) }; int ret; + if (2 + len > sizeof(buf)) { + printk(KERN_WARNING + "%s: i2c wr reg=%04x: len=%d is too big!\n", + KBUILD_MODNAME, reg, len); + return -EINVAL; + } + + buf[0] = MSB(reg); buf[1] = LSB(reg); memcpy(buf + 2, data, len); -- 1.8.3.1 -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 26/29] [media] af9035: Don't use dynamic static allocation
Dynamic static allocation is evil, as Kernel stack is too low, and compilation complains about it on some archs: drivers/media/usb/dvb-usb-v2/af9035.c:142:1: warning: 'af9035_wr_regs' uses dynamic stack allocation [enabled by default] drivers/media/usb/dvb-usb-v2/af9035.c:305:1: warning: 'af9035_i2c_master_xfer' uses dynamic stack allocation [enabled by default] Instead, let's enforce a limit for the buffer to be the max size of a control URB payload data (64 bytes). Signed-off-by: Mauro Carvalho Chehab --- drivers/media/usb/dvb-usb-v2/af9035.c | 29 ++--- 1 file changed, 26 insertions(+), 3 deletions(-) diff --git a/drivers/media/usb/dvb-usb-v2/af9035.c b/drivers/media/usb/dvb-usb-v2/af9035.c index 1ea17dc2a76e..c8fcd78425bd 100644 --- a/drivers/media/usb/dvb-usb-v2/af9035.c +++ b/drivers/media/usb/dvb-usb-v2/af9035.c @@ -21,6 +21,9 @@ #include "af9035.h" +/* Max transfer size done by I2C transfer functions */ +#define MAX_XFER_SIZE 64 + DVB_DEFINE_MOD_OPT_ADAPTER_NR(adapter_nr); static u16 af9035_checksum(const u8 *buf, size_t len) @@ -126,10 +129,16 @@ exit: /* write multiple registers */ static int af9035_wr_regs(struct dvb_usb_device *d, u32 reg, u8 *val, int len) { - u8 wbuf[6 + len]; + u8 wbuf[MAX_XFER_SIZE]; u8 mbox = (reg >> 16) & 0xff; struct usb_req req = { CMD_MEM_WR, mbox, sizeof(wbuf), wbuf, 0, NULL }; + if (6 + len > sizeof(wbuf)) { + dev_warn(&d->udev->dev, "%s: i2c wr: len=%d is too big!\n", +KBUILD_MODNAME, len); + return -EOPNOTSUPP; + } + wbuf[0] = len; wbuf[1] = 2; wbuf[2] = 0; @@ -228,9 +237,16 @@ static int af9035_i2c_master_xfer(struct i2c_adapter *adap, msg[1].len); } else { /* I2C */ - u8 buf[5 + msg[0].len]; + u8 buf[MAX_XFER_SIZE]; struct usb_req req = { CMD_I2C_RD, 0, sizeof(buf), buf, msg[1].len, msg[1].buf }; + + if (5 + msg[0].len > sizeof(buf)) { + dev_warn(&d->udev->dev, +"%s: i2c xfer: len=%d is too big!\n", +KBUILD_MODNAME, msg[0].len); + return -EOPNOTSUPP; + } req.mbox |= ((msg[0].addr & 0x80) >> 3); buf[0] = msg[1].len; buf[1] = msg[0].addr << 1; @@ -257,9 +273,16 @@ static int af9035_i2c_master_xfer(struct i2c_adapter *adap, msg[0].len - 3); } else { /* I2C */ - u8 buf[5 + msg[0].len]; + u8 buf[MAX_XFER_SIZE]; struct usb_req req = { CMD_I2C_WR, 0, sizeof(buf), buf, 0, NULL }; + + if (5 + msg[0].len > sizeof(buf)) { + dev_warn(&d->udev->dev, +"%s: i2c xfer: len=%d is too big!\n", +KBUILD_MODNAME, msg[0].len); + return -EOPNOTSUPP; + } req.mbox |= ((msg[0].addr & 0x80) >> 3); buf[0] = msg[0].len; buf[1] = msg[0].addr << 1; -- 1.8.3.1 -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 02/29] [media] radio-shark: remove a warning when CONFIG_PM is not defined
On alpha, allyesconfig doesn't have CONFIG_PM, and produces the following warnings: drivers/media/radio/radio-shark.c:274:13: warning: 'shark_resume_leds' defined but not used [-Wunused-function] drivers/media/radio/radio-shark2.c:240:13: warning: 'shark_resume_leds' defined but not used [-Wunused-function] That's because those functions are used only at device resume. Signed-off-by: Mauro Carvalho Chehab --- drivers/media/radio/radio-shark.c | 2 ++ drivers/media/radio/radio-shark2.c | 2 ++ 2 files changed, 4 insertions(+) diff --git a/drivers/media/radio/radio-shark.c b/drivers/media/radio/radio-shark.c index b91477212413..3db8a8cfe1a8 100644 --- a/drivers/media/radio/radio-shark.c +++ b/drivers/media/radio/radio-shark.c @@ -271,6 +271,7 @@ static void shark_unregister_leds(struct shark_device *shark) cancel_work_sync(&shark->led_work); } +#ifdef CONFIG_PM static void shark_resume_leds(struct shark_device *shark) { if (test_bit(BLUE_IS_PULSE, &shark->brightness_new)) @@ -280,6 +281,7 @@ static void shark_resume_leds(struct shark_device *shark) set_bit(RED_LED, &shark->brightness_new); schedule_work(&shark->led_work); } +#endif #else static int shark_register_leds(struct shark_device *shark, struct device *dev) { diff --git a/drivers/media/radio/radio-shark2.c b/drivers/media/radio/radio-shark2.c index 9fb669721e66..d86d90dab8bf 100644 --- a/drivers/media/radio/radio-shark2.c +++ b/drivers/media/radio/radio-shark2.c @@ -237,6 +237,7 @@ static void shark_unregister_leds(struct shark_device *shark) cancel_work_sync(&shark->led_work); } +#ifdef CONFIG_PM static void shark_resume_leds(struct shark_device *shark) { int i; @@ -246,6 +247,7 @@ static void shark_resume_leds(struct shark_device *shark) schedule_work(&shark->led_work); } +#endif #else static int shark_register_leds(struct shark_device *shark, struct device *dev) { -- 1.8.3.1 -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 21/29] [media] v4l2-async: Don't use dynamic static allocation
Dynamic static allocation is evil, as Kernel stack is too low, and compilation complains about it on some archs: drivers/media/v4l2-core/v4l2-async.c:238:1: warning: 'v4l2_async_notifier_unregister' uses dynamic stack allocation [enabled by default] Instead, let's enforce a limit for the buffer. In this specific case, there's a hard limit imposed by V4L2_MAX_SUBDEVS, with is currently 128. That means that the buffer size can be up to 128x8 = 1024 bytes (on a 64bits kernel), with is too big for stack. Worse than that, someone could increase it and cause real troubles. So, let's use dynamically allocated data, instead. Signed-off-by: Mauro Carvalho Chehab --- drivers/media/v4l2-core/v4l2-async.c | 31 --- 1 file changed, 28 insertions(+), 3 deletions(-) diff --git a/drivers/media/v4l2-core/v4l2-async.c b/drivers/media/v4l2-core/v4l2-async.c index c85d69da35bd..b56c9f300ecb 100644 --- a/drivers/media/v4l2-core/v4l2-async.c +++ b/drivers/media/v4l2-core/v4l2-async.c @@ -189,23 +189,41 @@ void v4l2_async_notifier_unregister(struct v4l2_async_notifier *notifier) struct v4l2_subdev *sd, *tmp; unsigned int notif_n_subdev = notifier->num_subdevs; unsigned int n_subdev = min(notif_n_subdev, V4L2_MAX_SUBDEVS); - struct device *dev[n_subdev]; + struct device **dev; int i = 0; if (!notifier->v4l2_dev) return; + dev = kmalloc(n_subdev * sizeof(*dev), GFP_KERNEL); + if (!dev) { + dev_err(notifier->v4l2_dev->dev, + "Failed to allocate device cache!\n"); + } + mutex_lock(&list_lock); list_del(¬ifier->list); list_for_each_entry_safe(sd, tmp, ¬ifier->done, async_list) { - dev[i] = get_device(sd->dev); + struct device *d; + + d = get_device(sd->dev); v4l2_async_cleanup(sd); /* If we handled USB devices, we'd have to lock the parent too */ - device_release_driver(dev[i++]); + device_release_driver(d); + + + /* +* Store device at the device cache, in order to call +* put_device() on the final step +*/ + if (dev) + dev[i++] = d; + else + put_device(d); if (notifier->unbind) notifier->unbind(notifier, sd, sd->asd); @@ -213,6 +231,12 @@ void v4l2_async_notifier_unregister(struct v4l2_async_notifier *notifier) mutex_unlock(&list_lock); + /* +* Call device_attach() to reprobe devices +* +* NOTE: If dev allocation fails, i is 0, and the hole loop won't be +* executed. +*/ while (i--) { struct device *d = dev[i]; @@ -228,6 +252,7 @@ void v4l2_async_notifier_unregister(struct v4l2_async_notifier *notifier) } put_device(d); } + kfree(dev); notifier->v4l2_dev = NULL; -- 1.8.3.1 -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 18/29] [media] tuners: Don't use dynamic static allocation
Dynamic static allocation is evil, as Kernel stack is too low, and compilation complains about it on some archs: drivers/media/tuners/e4000.c:50:1: warning: 'e4000_wr_regs' uses dynamic stack allocation [enabled by default] drivers/media/tuners/e4000.c:83:1: warning: 'e4000_rd_regs' uses dynamic stack allocation [enabled by default] drivers/media/tuners/fc2580.c:66:1: warning: 'fc2580_wr_regs.constprop.1' uses dynamic stack allocation [enabled by default] drivers/media/tuners/fc2580.c:98:1: warning: 'fc2580_rd_regs.constprop.0' uses dynamic stack allocation [enabled by default] drivers/media/tuners/tda18212.c:57:1: warning: 'tda18212_wr_regs' uses dynamic stack allocation [enabled by default] drivers/media/tuners/tda18212.c:90:1: warning: 'tda18212_rd_regs.constprop.0' uses dynamic stack allocation [enabled by default] drivers/media/tuners/tda18218.c:60:1: warning: 'tda18218_wr_regs' uses dynamic stack allocation [enabled by default] drivers/media/tuners/tda18218.c:92:1: warning: 'tda18218_rd_regs.constprop.0' uses dynamic stack allocation [enabled by default] Instead, let's enforce a limit for the buffer. Considering that I2C transfers are generally limited, and that devices used on USB has a max data length of 64 bytes for the control URBs. So, it seem safe to use 64 bytes as the hard limit for all those devices. On most cases, the limit is a way lower than that, but this limit is small enough to not affect the Kernel stack, and it is a no brain limit, as using smaller ones would require to either carefully each driver or to take a look on each datasheet. Signed-off-by: Mauro Carvalho Chehab --- drivers/media/tuners/e4000.c| 21 +++-- drivers/media/tuners/fc2580.c | 21 +++-- drivers/media/tuners/tda18212.c | 21 +++-- drivers/media/tuners/tda18218.c | 21 +++-- 4 files changed, 76 insertions(+), 8 deletions(-) diff --git a/drivers/media/tuners/e4000.c b/drivers/media/tuners/e4000.c index ad9309da4a91..30192463c9e1 100644 --- a/drivers/media/tuners/e4000.c +++ b/drivers/media/tuners/e4000.c @@ -20,11 +20,14 @@ #include "e4000_priv.h" +/* Max transfer size done by I2C transfer functions */ +#define MAX_XFER_SIZE 64 + /* write multiple registers */ static int e4000_wr_regs(struct e4000_priv *priv, u8 reg, u8 *val, int len) { int ret; - u8 buf[1 + len]; + u8 buf[MAX_XFER_SIZE]; struct i2c_msg msg[1] = { { .addr = priv->cfg->i2c_addr, @@ -34,6 +37,13 @@ static int e4000_wr_regs(struct e4000_priv *priv, u8 reg, u8 *val, int len) } }; + if (1 + len > sizeof(buf)) { + dev_warn(&priv->i2c->dev, +"%s: i2c wr reg=%04x: len=%d is too big!\n", +KBUILD_MODNAME, reg, len); + return -EINVAL; + } + buf[0] = reg; memcpy(&buf[1], val, len); @@ -53,7 +63,7 @@ static int e4000_wr_regs(struct e4000_priv *priv, u8 reg, u8 *val, int len) static int e4000_rd_regs(struct e4000_priv *priv, u8 reg, u8 *val, int len) { int ret; - u8 buf[len]; + u8 buf[MAX_XFER_SIZE]; struct i2c_msg msg[2] = { { .addr = priv->cfg->i2c_addr, @@ -68,6 +78,13 @@ static int e4000_rd_regs(struct e4000_priv *priv, u8 reg, u8 *val, int len) } }; + if (len > sizeof(buf)) { + dev_warn(&priv->i2c->dev, +"%s: i2c rd reg=%04x: len=%d is too big!\n", +KBUILD_MODNAME, reg, len); + return -EINVAL; + } + ret = i2c_transfer(priv->i2c, msg, 2); if (ret == 2) { memcpy(val, buf, len); diff --git a/drivers/media/tuners/fc2580.c b/drivers/media/tuners/fc2580.c index 81f38aae9c66..430fa5163ec7 100644 --- a/drivers/media/tuners/fc2580.c +++ b/drivers/media/tuners/fc2580.c @@ -20,6 +20,9 @@ #include "fc2580_priv.h" +/* Max transfer size done by I2C transfer functions */ +#define MAX_XFER_SIZE 64 + /* * TODO: * I2C write and read works only for one single register. Multiple registers @@ -41,7 +44,7 @@ static int fc2580_wr_regs(struct fc2580_priv *priv, u8 reg, u8 *val, int len) { int ret; - u8 buf[1 + len]; + u8 buf[MAX_XFER_SIZE]; struct i2c_msg msg[1] = { { .addr = priv->cfg->i2c_addr, @@ -51,6 +54,13 @@ static int fc2580_wr_regs(struct fc2580_priv *priv, u8 reg, u8 *val, int len) } }; + if (1 + len > sizeof(buf)) { + dev_warn(&priv->i2c->dev, +"%s: i2c wr reg=%04x: len=%d is too big!\n", +KBUILD_MODNAME, reg, len); + return -EINVAL; + } + buf[0] = reg; memcpy(&buf[1], val, len); @@ -69,7 +79,7
[PATCH v3 14/29] [media] stb0899_drv: Don't use dynamic static allocation
Dynamic static allocation is evil, as Kernel stack is too low, and compilation complains about it on some archs: drivers/media/dvb-frontends/stb0899_drv.c:540:1: warning: 'stb0899_write_regs' uses dynamic stack allocation [enabled by default] Instead, let's enforce a limit for the buffer. Considering that I2C transfers are generally limited, and that devices used on USB has a max data length of 64 bytes for the control URBs. So, it seem safe to use 64 bytes as the hard limit for all those devices. On most cases, the limit is a way lower than that, but this limit is small enough to not affect the Kernel stack, and it is a no brain limit, as using smaller ones would require to either carefully each driver or to take a look on each datasheet. Signed-off-by: Mauro Carvalho Chehab --- drivers/media/dvb-frontends/stb0899_drv.c | 12 +++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/drivers/media/dvb-frontends/stb0899_drv.c b/drivers/media/dvb-frontends/stb0899_drv.c index 3dd5714eadba..07cd5ea7a038 100644 --- a/drivers/media/dvb-frontends/stb0899_drv.c +++ b/drivers/media/dvb-frontends/stb0899_drv.c @@ -32,6 +32,9 @@ #include "stb0899_priv.h" #include "stb0899_reg.h" +/* Max transfer size done by I2C transfer functions */ +#define MAX_XFER_SIZE 64 + static unsigned int verbose = 0;//1; module_param(verbose, int, 0644); @@ -499,7 +502,7 @@ err: int stb0899_write_regs(struct stb0899_state *state, unsigned int reg, u8 *data, u32 count) { int ret; - u8 buf[2 + count]; + u8 buf[MAX_XFER_SIZE]; struct i2c_msg i2c_msg = { .addr = state->config->demod_address, .flags = 0, @@ -507,6 +510,13 @@ int stb0899_write_regs(struct stb0899_state *state, unsigned int reg, u8 *data, .len= 2 + count }; + if (2 + count > sizeof(buf)) { + printk(KERN_WARNING + "%s: i2c wr reg=%04x: len=%d is too big!\n", + KBUILD_MODNAME, reg, count); + return -EINVAL; + } + buf[0] = reg >> 8; buf[1] = reg & 0xff; memcpy(&buf[2], data, count); -- 1.8.3.1 -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 06/29] [media] iguanair: simplify calculation of carrier delay cycles
From: Sean Young Simplify the logic that calculates the carrier, and removes a warning on avr32 arch: drivers/media/rc/iguanair.c: In function 'iguanair_set_tx_carrier': drivers/media/rc/iguanair.c:304: warning: 'sevens' may be used uninitialized in this function Signed-off-by: Sean Young Signed-off-by: Mauro Carvalho Chehab --- drivers/media/rc/iguanair.c | 22 ++ 1 file changed, 6 insertions(+), 16 deletions(-) diff --git a/drivers/media/rc/iguanair.c b/drivers/media/rc/iguanair.c index 19632b1c2190..7f05e197680b 100644 --- a/drivers/media/rc/iguanair.c +++ b/drivers/media/rc/iguanair.c @@ -308,22 +308,12 @@ static int iguanair_set_tx_carrier(struct rc_dev *dev, uint32_t carrier) cycles = DIV_ROUND_CLOSEST(2400, carrier * 2) - ir->cycle_overhead; - /* make up the the remainer of 4-cycle blocks */ - switch (cycles & 3) { - case 0: - sevens = 0; - break; - case 1: - sevens = 3; - break; - case 2: - sevens = 2; - break; - case 3: - sevens = 1; - break; - } - + /* +* Calculate minimum number of 7 cycles needed so +* we are left with a multiple of 4; so we want to have +* (sevens * 7) & 3 == cycles & 3 +*/ + sevens = (4 - cycles) & 3; fours = (cycles - sevens * 7) / 4; /* magic happens here */ -- 1.8.3.1 -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v11 03/12] [media] exynos5-fimc-is: Add common driver header files
Hi Arun, On Tue, Nov 05, 2013 at 11:42:34AM +0530, Arun Kumar K wrote: > This patch adds all the common header files used by the fimc-is > driver. It includes the commands for interfacing with the firmware > and error codes from IS firmware, metadata and command parameter > definitions. > > Signed-off-by: Arun Kumar K > Signed-off-by: Kilyeon Im > Reviewed-by: Sylwester Nawrocki > --- > drivers/media/platform/exynos5-is/fimc-is-cmd.h| 187 > drivers/media/platform/exynos5-is/fimc-is-err.h| 257 + > .../media/platform/exynos5-is/fimc-is-metadata.h | 767 + > drivers/media/platform/exynos5-is/fimc-is-param.h | 1159 > > 4 files changed, 2370 insertions(+) > create mode 100644 drivers/media/platform/exynos5-is/fimc-is-cmd.h > create mode 100644 drivers/media/platform/exynos5-is/fimc-is-err.h > create mode 100644 drivers/media/platform/exynos5-is/fimc-is-metadata.h > create mode 100644 drivers/media/platform/exynos5-is/fimc-is-param.h > > diff --git a/drivers/media/platform/exynos5-is/fimc-is-cmd.h > b/drivers/media/platform/exynos5-is/fimc-is-cmd.h > new file mode 100644 > index 000..6250280 > --- /dev/null > +++ b/drivers/media/platform/exynos5-is/fimc-is-cmd.h > @@ -0,0 +1,187 @@ > +/* > + * Samsung Exynos5 SoC series FIMC-IS driver > + * > + * Copyright (c) 2013 Samsung Electronics Co., Ltd > + * Kil-yeon Lim > + * > + * 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. > + */ > + > +#ifndef FIMC_IS_CMD_H > +#define FIMC_IS_CMD_H > + > +#define IS_COMMAND_VER 122 /* IS COMMAND VERSION 1.22 */ > + > +enum is_cmd { > + /* HOST -> IS */ > + HIC_PREVIEW_STILL = 0x1, > + HIC_PREVIEW_VIDEO, > + HIC_CAPTURE_STILL, > + HIC_CAPTURE_VIDEO, > + HIC_PROCESS_START, > + HIC_PROCESS_STOP, > + HIC_STREAM_ON, > + HIC_STREAM_OFF, > + HIC_SHOT, > + HIC_GET_STATIC_METADATA, > + HIC_SET_CAM_CONTROL, > + HIC_GET_CAM_CONTROL, > + HIC_SET_PARAMETER, > + HIC_GET_PARAMETER, > + HIC_SET_A5_MEM_ACCESS, > + RESERVED2, > + HIC_GET_STATUS, > + /* SENSOR PART*/ > + HIC_OPEN_SENSOR, > + HIC_CLOSE_SENSOR, > + HIC_SIMMIAN_INIT, > + HIC_SIMMIAN_WRITE, > + HIC_SIMMIAN_READ, > + HIC_POWER_DOWN, > + HIC_GET_SET_FILE_ADDR, > + HIC_LOAD_SET_FILE, > + HIC_MSG_CONFIG, > + HIC_MSG_TEST, > + /* IS -> HOST */ > + IHC_GET_SENSOR_NUMBER = 0x1000, > + /* Parameter1 : Address of space to copy a setfile */ > + /* Parameter2 : Space szie */ > + IHC_SET_SHOT_MARK, > + /* PARAM1 : a frame number */ > + /* PARAM2 : confidence level(smile 0~100) */ > + /* PARMA3 : confidence level(blink 0~100) */ > + IHC_SET_FACE_MARK, > + /* PARAM1 : coordinate count */ > + /* PARAM2 : coordinate buffer address */ > + IHC_FRAME_DONE, > + /* PARAM1 : frame start number */ > + /* PARAM2 : frame count */ > + IHC_AA_DONE, > + IHC_NOT_READY, > + IHC_FLASH_READY > +}; > + > +enum is_reply { > + ISR_DONE= 0x2000, > + ISR_NDONE > +}; > + > +enum is_scenario_id { > + ISS_PREVIEW_STILL, > + ISS_PREVIEW_VIDEO, > + ISS_CAPTURE_STILL, > + ISS_CAPTURE_VIDEO, > + ISS_END > +}; > + > +enum is_subscenario_id { > + ISS_SUB_SCENARIO_STILL, > + ISS_SUB_SCENARIO_VIDEO, > + ISS_SUB_SCENARIO_SCENE1, > + ISS_SUB_SCENARIO_SCENE2, > + ISS_SUB_SCENARIO_SCENE3, > + ISS_SUB_END > +}; > + > +struct is_setfile_header_element { > + u32 binary_addr; > + u32 binary_size; > +}; > + > +struct is_setfile_header { > + struct is_setfile_header_element isp[ISS_END]; > + struct is_setfile_header_element drc[ISS_END]; > + struct is_setfile_header_element fd[ISS_END]; > +}; > + > +struct is_common_reg { > + u32 hicmd; > + u32 hic_sensorid; > + u32 hic_param[4]; > + > + u32 reserved1[3]; > + > + u32 ihcmd_iflag; > + u32 ihcmd; > + u32 ihc_sensorid; > + u32 ihc_param[4]; > + > + u32 reserved2[3]; > + > + u32 isp_bayer_iflag; > + u32 isp_bayer_sensor_id; > + u32 isp_bayer_param[2]; > + > + u32 reserved3[4]; > + > + u32 scc_iflag; > + u32 scc_sensor_id; > + u32 scc_param[3]; > + > + u32 reserved4[3]; > + > + u32 dnr_iflag; > + u32 dnr_sensor_id; > + u32 dnr_param[2]; > + > + u32 reserved5[4]; > + > + u32 scp_iflag; > + u32 scp_sensor_id; > + u32 scp_param[3]; > + > + u32 reserved6[1]; > + > + u32 isp_yuv_iflag; > + u32 isp_yuv_sensor_id; > + u32 isp_yuv_param[2]; > + > + u32 reserved7[1]; > + > + u32 shot_iflag; > + u32 shot_sensor_id; > + u32 shot_param[2]; > + > + u32 reserved8[1]; > + > + u32 meta_iflag; > + u32 meta_sensor_id; > + u32 meta_param1; > + > + u32 reserved9[1]; > + > + u32 fcount; If thes
Re: [PATCHv2 22/29] v4l2-async: Don't use dynamic static allocation
On 11/05/13 13:03, Mauro Carvalho Chehab wrote: > Em Tue, 05 Nov 2013 12:42:23 +0100 > Sylwester Nawrocki escreveu: > >> On 05/11/13 12:36, Mauro Carvalho Chehab wrote: > diff --git a/drivers/media/v4l2-core/v4l2-async.c > b/drivers/media/v4l2-core/v4l2-async.c >>> index c85d69da35bd..071596869036 100644 >>> --- a/drivers/media/v4l2-core/v4l2-async.c >>> +++ b/drivers/media/v4l2-core/v4l2-async.c >>> @@ -189,12 +189,14 @@ void v4l2_async_notifier_unregister(struct >>> v4l2_async_notifier *notifier) >>> struct v4l2_subdev *sd, *tmp; >>> unsigned int notif_n_subdev = notifier->num_subdevs; >>> unsigned int n_subdev = min(notif_n_subdev, V4L2_MAX_SUBDEVS); >>> - struct device *dev[n_subdev]; >>> + struct device **dev; >>> int i = 0; >>> >>> if (!notifier->v4l2_dev) >>> return; >>> >>> + dev = kmalloc(sizeof(*dev) * n_subdev, GFP_KERNEL); >>> + > > No check for dev == NULL? >>> Well, what should be done in this case? >>> >>> We could do the changes below: >>> >>> void v4l2_async_notifier_unregister(struct v4l2_async_notifier *notifier) >>> { >>> struct v4l2_subdev *sd, *tmp; >>> unsigned int notif_n_subdev = notifier->num_subdevs; >>> unsigned int n_subdev = min(notif_n_subdev, V4L2_MAX_SUBDEVS); >>> - struct device *dev[n_subdev]; >>> + struct device **dev; >>> int i = 0; >>> >>> if (!notifier->v4l2_dev) >>> return; >>> >>> + dev = kmalloc(sizeof(*dev) * n_subdev, GFP_KERNEL); >>> + if (!dev) { >>> + WARN_ON(true); >>> + return; >>> + } >>> + >>> mutex_lock(&list_lock); >>> >>> list_del(¬ifier->list); >>> >>> list_for_each_entry_safe(sd, tmp, ¬ifier->done, async_list) { >>> dev[i] = get_device(sd->dev); >>> >>> v4l2_async_cleanup(sd); >>> >>> /* If we handled USB devices, we'd have to lock the parent >>> too */ >>> device_release_driver(dev[i++]); >>> >>> if (notifier->unbind) >>> notifier->unbind(notifier, sd, sd->asd); >>> } >>> >>> mutex_unlock(&list_lock); >>> >>> while (i--) { >>> struct device *d = dev[i]; >>> >>> if (d && device_attach(d) < 0) { >>> const char *name = "(none)"; >>> int lock = device_trylock(d); >>> >>> if (lock && d->driver) >>> name = d->driver->name; >>> dev_err(d, "Failed to re-probe to %s\n", name); >>> if (lock) >>> device_unlock(d); >>> } >>> put_device(d); >>> } >>> + kfree(dev); >>> >>> notifier->v4l2_dev = NULL; >>> >>> /* >>> * Don't care about the waiting list, it is initialised and >>> populated >>> * upon notifier registration. >>> */ >>> } >>> EXPORT_SYMBOL(v4l2_async_notifier_unregister); >>> >>> But I suspect that this will cause an OOPS anyway, as the device will be >>> only half-removed. So, it would likely OOPS at device removal or if the >>> device got probed again, at probing time. >>> >>> So, IMHO, we should have at least a WARN_ON() for this case. >>> >>> Do you have a better idea? >> >> This is how Guennadi's patch looked like when it used dynamic allocation: >> >> http://www.spinics.net/lists/linux-sh/msg18194.html > > Thanks for the tip! > > The following patch should do the trick (generated with -U10, in order > to show the entire function): > > [PATCHv3] v4l2-async: Don't use dynamic static allocation > > Dynamic static allocation is evil, as Kernel stack is too low, and > compilation complains about it on some archs: > > drivers/media/v4l2-core/v4l2-async.c:238:1: warning: > 'v4l2_async_notifier_unregister' uses dynamic stack allocation [enabled by > default] > > Instead, let's enforce a limit for the buffer. > > In this specific case, there's a hard limit imposed by V4L2_MAX_SUBDEVS, > with is currently 128. That means that the buffer size can be up to > 128x8 = 1024 bytes (on a 64bits kernel), with is too big for stack. > > Worse than that, someone could increase it and cause real troubles. > > So, let's use dynamically allocated data, instead. > > Signed-off-by: Mauro Carvalho Chehab > Cc: Guennadi Liakhovetski > > diff --git a/drivers/media/v4l2-core/v4l2-async.c > b/drivers/media/v4l2-core/v4l2-async.c > index c85d69da35bd..b56c9f300ecb 100644 > --- a/drivers/media/v4l2-core/v4l2-async.c > +++ b/drivers/media/v4l2-core/v4l2-async.c > @@ -182,59 +182,84 @@ int v4l2_async_notifier_register(struct v4l2_device > *v4l2_dev, > > return 0; > } > EXPORT_SYMBOL(v4l2_async_no
[PATCH v12 08/12] [media] exynos5-fimc-is: Add the hardware pipeline control
This patch adds the crucial hardware pipeline control for the fimc-is driver. All the subdev nodes will call this pipeline interfaces to reach the hardware. Responsibilities of this module involves configuring and maintaining the hardware pipeline involving multiple sub-ips like ISP, DRC, Scalers, ODC, 3DNR, FD etc. Signed-off-by: Arun Kumar K Signed-off-by: Kilyeon Im Reviewed-by: Sylwester Nawrocki --- .../media/platform/exynos5-is/fimc-is-pipeline.c | 1699 .../media/platform/exynos5-is/fimc-is-pipeline.h | 129 ++ 2 files changed, 1828 insertions(+) create mode 100644 drivers/media/platform/exynos5-is/fimc-is-pipeline.c create mode 100644 drivers/media/platform/exynos5-is/fimc-is-pipeline.h diff --git a/drivers/media/platform/exynos5-is/fimc-is-pipeline.c b/drivers/media/platform/exynos5-is/fimc-is-pipeline.c new file mode 100644 index 000..25eaf24 --- /dev/null +++ b/drivers/media/platform/exynos5-is/fimc-is-pipeline.c @@ -0,0 +1,1699 @@ +/* + * Samsung EXYNOS5 FIMC-IS (Imaging Subsystem) driver +* + * Copyright (C) 2013 Samsung Electronics Co., Ltd. + * Arun Kumar K + * Kil-yeon Lim + * + * 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 "fimc-is.h" +#include "fimc-is-pipeline.h" +#include "fimc-is-metadata.h" +#include "fimc-is-regs.h" +#include "fimc-is-cmd.h" +#include +#include + +/* Default setting values */ +#define DEFAULT_PREVIEW_STILL_WIDTH1280 +#define DEFAULT_PREVIEW_STILL_HEIGHT 720 +#define DEFAULT_CAPTURE_VIDEO_WIDTH1920 +#define DEFAULT_CAPTURE_VIDEO_HEIGHT 1080 +#define DEFAULT_CAPTURE_STILL_WIDTH2560 +#define DEFAULT_CAPTURE_STILL_HEIGHT 1920 +#define DEFAULT_CAPTURE_STILL_CROP_WIDTH 2560 +#define DEFAULT_CAPTURE_STILL_CROP_HEIGHT 1440 +#define DEFAULT_PREVIEW_VIDEO_WIDTH640 +#define DEFAULT_PREVIEW_VIDEO_HEIGHT 480 + +/* Init params for pipeline devices */ +static const struct sensor_param init_sensor_param = { + .frame_rate = { + .frame_rate = 30, + }, +}; + +static const struct isp_param init_isp_param = { + .control = { + .cmd = CONTROL_COMMAND_START, + .bypass = CONTROL_BYPASS_DISABLE, + }, + .otf_input = { + .cmd = OTF_INPUT_COMMAND_DISABLE, + .width = DEFAULT_CAPTURE_STILL_WIDTH, + .height = DEFAULT_CAPTURE_STILL_HEIGHT, + .format = OTF_INPUT_FORMAT_BAYER, + .bitwidth = OTF_INPUT_BIT_WIDTH_10BIT, + .order = OTF_INPUT_ORDER_BAYER_GR_BG, + .frametime_max = 3, + }, + .dma1_input = { + .cmd = DMA_INPUT_COMMAND_DISABLE, + }, + .dma2_input = { + .cmd = DMA_INPUT_COMMAND_DISABLE, + }, + .aa = { + .cmd = ISP_AA_COMMAND_START, + }, + .flash = { + .cmd = ISP_FLASH_COMMAND_DISABLE, + .redeye = ISP_FLASH_REDEYE_DISABLE, + }, + .awb = { + .cmd = ISP_AWB_COMMAND_AUTO, + }, + .effect = { + .cmd = ISP_IMAGE_EFFECT_DISABLE, + }, + .iso = { + .cmd = ISP_ISO_COMMAND_AUTO, + }, + .adjust = { + .cmd = ISP_ADJUST_COMMAND_AUTO, + }, + .metering = { + .cmd = ISP_METERING_COMMAND_CENTER, + .win_width = DEFAULT_CAPTURE_STILL_WIDTH, + .win_height = DEFAULT_CAPTURE_STILL_HEIGHT, + }, + .afc = { + .cmd = ISP_AFC_COMMAND_AUTO, + }, + .otf_output = { + .cmd = OTF_OUTPUT_COMMAND_ENABLE, + .width = DEFAULT_CAPTURE_STILL_WIDTH, + .height = DEFAULT_CAPTURE_STILL_HEIGHT, + .format = OTF_OUTPUT_FORMAT_YUV444, + .bitwidth = OTF_OUTPUT_BIT_WIDTH_12BIT, + .order = OTF_OUTPUT_ORDER_BAYER_GR_BG, + }, + .dma1_output = { + .cmd = DMA_OUTPUT_COMMAND_DISABLE, + .width = DEFAULT_CAPTURE_STILL_WIDTH, + .height = DEFAULT_CAPTURE_STILL_HEIGHT, + .format = DMA_INPUT_FORMAT_YUV444, + .bitwidth = DMA_INPUT_BIT_WIDTH_8BIT, + .plane = 1, + .order = DMA_INPUT_ORDER_YCBCR, + }, + .dma2_output = { + .cmd = DMA_OUTPUT_COMMAND_DISABLE, + .width = DEFAULT_CAPTURE_STILL_WIDTH, + .height = DEFAULT_CAPTURE_STILL_HEIGHT, + .format = DMA_OUTPUT_FORMAT_BAYER, + .bitwidth = DMA_OUTPUT_BIT_WIDTH_12BIT, + .plane = 1, + .order = DMA_OUTPUT_ORDER_GB_BG, + .dma_out_mask = 0x, + }, +}; + +static const struct drc_param init_drc_param = { + .control
[PATCH v12 11/12] V4L: Add DT binding doc for s5k4e5 image sensor
S5K4E5 is a Samsung raw image sensor controlled via I2C. This patch adds the DT binding documentation for the same. Signed-off-by: Arun Kumar K Reviewed-by: Sylwester Nawrocki Acked-by: Mark Rutland --- .../devicetree/bindings/media/samsung-s5k4e5.txt | 45 1 file changed, 45 insertions(+) create mode 100644 Documentation/devicetree/bindings/media/samsung-s5k4e5.txt diff --git a/Documentation/devicetree/bindings/media/samsung-s5k4e5.txt b/Documentation/devicetree/bindings/media/samsung-s5k4e5.txt new file mode 100644 index 000..fc37792 --- /dev/null +++ b/Documentation/devicetree/bindings/media/samsung-s5k4e5.txt @@ -0,0 +1,45 @@ +* Samsung S5K4E5 Raw Image Sensor + +S5K4E5 is a raw image sensor with maximum resolution of 2560x1920 +pixels. Data transfer is carried out via MIPI CSI-2 port and controls +via I2C bus. + +Required Properties: +- compatible : should contain "samsung,s5k4e5" +- reg : I2C device address +- reset-gpios : specifier of a GPIO connected to the RESET pin +- clocks : should refer to the clock named in clock-names, from + the common clock bindings +- clock-names : should contain "extclk" entry +- svdda-supply : core voltage supply +- svddio-supply: I/O voltage supply + +Optional Properties: +- clock-frequency : the frequency at which the "extclk" clock should be + configured to operate, in Hz; if this property is not + specified default 24 MHz value will be used + +The device node should be added to respective control bus controller +(e.g. I2C0) nodes and linked to the csis port node, using the common +video interfaces bindings, defined in video-interfaces.txt. + +Example: + + i2c-isp@1313 { + s5k4e5@20 { + compatible = "samsung,s5k4e5"; + reg = <0x20>; + reset-gpios = <&gpx1 2 1>; + clock-frequency = <2400>; + clocks = <&clock 129>; + clock-names = "extclk" + svdda-supply = <...>; + svddio-supply = <...>; + port { + is_s5k4e5_ep: endpoint { + data-lanes = <1 2 3 4>; + remote-endpoint = <&csis0_ep>; + }; + }; + }; + }; -- 1.7.9.5 -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v12 10/12] [media] exynos5-is: Add Kconfig and Makefile
Adds Kconfig and Makefile for exynos5-is driver files. Signed-off-by: Shaik Ameer Basha Signed-off-by: Arun Kumar K Reviewed-by: Sylwester Nawrocki --- drivers/media/platform/Kconfig |1 + drivers/media/platform/Makefile |1 + drivers/media/platform/exynos5-is/Kconfig| 20 drivers/media/platform/exynos5-is/Makefile |7 +++ drivers/media/platform/exynos5-is/fimc-is-core.c |1 + 5 files changed, 30 insertions(+) create mode 100644 drivers/media/platform/exynos5-is/Kconfig create mode 100644 drivers/media/platform/exynos5-is/Makefile diff --git a/drivers/media/platform/Kconfig b/drivers/media/platform/Kconfig index 08de865..4b0475e 100644 --- a/drivers/media/platform/Kconfig +++ b/drivers/media/platform/Kconfig @@ -123,6 +123,7 @@ config VIDEO_S3C_CAMIF source "drivers/media/platform/soc_camera/Kconfig" source "drivers/media/platform/exynos4-is/Kconfig" +source "drivers/media/platform/exynos5-is/Kconfig" source "drivers/media/platform/s5p-tv/Kconfig" endif # V4L_PLATFORM_DRIVERS diff --git a/drivers/media/platform/Makefile b/drivers/media/platform/Makefile index eee28dd..40bf09f 100644 --- a/drivers/media/platform/Makefile +++ b/drivers/media/platform/Makefile @@ -37,6 +37,7 @@ obj-$(CONFIG_VIDEO_SAMSUNG_S5P_TV)+= s5p-tv/ obj-$(CONFIG_VIDEO_SAMSUNG_S5P_G2D)+= s5p-g2d/ obj-$(CONFIG_VIDEO_SAMSUNG_EXYNOS_GSC) += exynos-gsc/ +obj-$(CONFIG_VIDEO_SAMSUNG_EXYNOS5_CAMERA) += exynos5-is/ obj-$(CONFIG_BLACKFIN) += blackfin/ diff --git a/drivers/media/platform/exynos5-is/Kconfig b/drivers/media/platform/exynos5-is/Kconfig new file mode 100644 index 000..b67d11a --- /dev/null +++ b/drivers/media/platform/exynos5-is/Kconfig @@ -0,0 +1,20 @@ +config VIDEO_SAMSUNG_EXYNOS5_CAMERA + bool "Samsung Exynos5 SoC Camera Media Device driver" + depends on VIDEO_V4L2 && VIDEO_V4L2_SUBDEV_API && PM_RUNTIME + depends on VIDEO_SAMSUNG_EXYNOS4_IS + help + This is a V4L2 media device driver for Exynos5 SoC series + camera subsystem. + +if VIDEO_SAMSUNG_EXYNOS5_CAMERA + +config VIDEO_SAMSUNG_EXYNOS5_FIMC_IS + tristate "Samsung Exynos5 SoC FIMC-IS driver" + depends on I2C && OF + depends on VIDEO_EXYNOS4_FIMC_IS + select VIDEOBUF2_DMA_CONTIG + help + This is a V4L2 driver for Samsung Exynos5 SoC series Imaging + Subsystem known as FIMC-IS. + +endif #VIDEO_SAMSUNG_EXYNOS5_MDEV diff --git a/drivers/media/platform/exynos5-is/Makefile b/drivers/media/platform/exynos5-is/Makefile new file mode 100644 index 000..6cdb037 --- /dev/null +++ b/drivers/media/platform/exynos5-is/Makefile @@ -0,0 +1,7 @@ +ccflags-y += -Idrivers/media/platform/exynos4-is +exynos5-fimc-is-objs := fimc-is-core.o fimc-is-isp.o fimc-is-scaler.o +exynos5-fimc-is-objs += fimc-is-pipeline.o fimc-is-interface.o fimc-is-sensor.o +exynos-mdevice-objs := exynos5-mdev.o + +obj-$(CONFIG_VIDEO_SAMSUNG_EXYNOS5_FIMC_IS) += exynos5-fimc-is.o +obj-$(CONFIG_VIDEO_SAMSUNG_EXYNOS5_CAMERA) += exynos-mdevice.o diff --git a/drivers/media/platform/exynos5-is/fimc-is-core.c b/drivers/media/platform/exynos5-is/fimc-is-core.c index 136e3c1..0b391f2 100644 --- a/drivers/media/platform/exynos5-is/fimc-is-core.c +++ b/drivers/media/platform/exynos5-is/fimc-is-core.c @@ -12,6 +12,7 @@ #include #include #include +#include #include #include -- 1.7.9.5 -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v12 09/12] [media] exynos5-fimc-is: Add the hardware interface module
The hardware interface module finally sends the commands to the FIMC-IS firmware and runs the interrupt handler for getting the responses. Signed-off-by: Arun Kumar K Signed-off-by: Kilyeon Im Reviewed-by: Sylwester Nawrocki --- .../media/platform/exynos5-is/fimc-is-interface.c | 810 .../media/platform/exynos5-is/fimc-is-interface.h | 124 +++ 2 files changed, 934 insertions(+) create mode 100644 drivers/media/platform/exynos5-is/fimc-is-interface.c create mode 100644 drivers/media/platform/exynos5-is/fimc-is-interface.h diff --git a/drivers/media/platform/exynos5-is/fimc-is-interface.c b/drivers/media/platform/exynos5-is/fimc-is-interface.c new file mode 100644 index 000..c5da6ff --- /dev/null +++ b/drivers/media/platform/exynos5-is/fimc-is-interface.c @@ -0,0 +1,810 @@ +/* + * Samsung EXYNOS5 FIMC-IS (Imaging Subsystem) driver +* + * Copyright (C) 2013 Samsung Electronics Co., Ltd. + * Kil-yeon Lim + * + * 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 "fimc-is.h" +#include "fimc-is-cmd.h" +#include "fimc-is-regs.h" + +#define init_request_barrier(itf) mutex_init(&itf->request_barrier) +#define enter_request_barrier(itf) mutex_lock(&itf->request_barrier) +#define exit_request_barrier(itf) mutex_unlock(&itf->request_barrier) + +static inline void itf_get_cmd(struct fimc_is_interface *itf, + struct fimc_is_msg *msg, unsigned int index) +{ + struct is_common_reg __iomem *com_regs = itf->com_regs; + + memset(msg, 0, sizeof(*msg)); + + switch (index) { + case INTR_GENERAL: + msg->command = com_regs->ihcmd; + msg->instance = com_regs->ihc_sensorid; + memcpy(msg->param, com_regs->ihc_param, + 4 * sizeof(msg->param[0])); + break; + case INTR_SCC_FDONE: + msg->command = IHC_FRAME_DONE; + msg->instance = com_regs->scc_sensor_id; + memcpy(msg->param, com_regs->scc_param, + 3 * sizeof(msg->param[0])); + break; + case INTR_SCP_FDONE: + msg->command = IHC_FRAME_DONE; + msg->instance = com_regs->scp_sensor_id; + memcpy(msg->param, com_regs->scp_param, + 3 * sizeof(msg->param[0])); + break; + case INTR_META_DONE: + msg->command = IHC_FRAME_DONE; + msg->instance = com_regs->meta_sensor_id; + msg->param[0] = com_regs->meta_param1; + break; + case INTR_SHOT_DONE: + msg->command = IHC_FRAME_DONE; + msg->instance = com_regs->shot_sensor_id; + memcpy(msg->param, com_regs->shot_param, + 2 * sizeof(msg->param[0])); + break; + default: + dev_err(itf->dev, "%s Unknown command\n", __func__); + break; + } +} + +static inline unsigned int itf_get_intr(struct fimc_is_interface *itf) +{ + unsigned int status; + struct is_common_reg __iomem *com_regs = itf->com_regs; + + status = readl(itf->regs + INTMSR1) | com_regs->ihcmd_iflag | + com_regs->scc_iflag | + com_regs->scp_iflag | + com_regs->meta_iflag | + com_regs->shot_iflag; + + return status; +} + +static void itf_set_state(struct fimc_is_interface *itf, + unsigned long state) +{ + unsigned long flags; + spin_lock_irqsave(&itf->slock_state, flags); + __set_bit(state, &itf->state); + spin_unlock_irqrestore(&itf->slock_state, flags); +} + +static void itf_clr_state(struct fimc_is_interface *itf, + unsigned long state) +{ + unsigned long flags; + spin_lock_irqsave(&itf->slock_state, flags); + __clear_bit(state, &itf->state); + spin_unlock_irqrestore(&itf->slock_state, flags); +} + +static int itf_get_state(struct fimc_is_interface *itf, + unsigned long state) +{ + int ret = 0; + unsigned long flags; + + spin_lock_irqsave(&itf->slock_state, flags); + ret = test_bit(state, &itf->state); + spin_unlock_irqrestore(&itf->slock_state, flags); + return ret; +} + +static void itf_init_wakeup(struct fimc_is_interface *itf) +{ + itf_set_state(itf, IS_IF_STATE_INIT); + wake_up(&itf->irq_queue); +} + +void itf_busy_wakeup(struct fimc_is_interface *itf) +{ + itf_clr_state(itf, IS_IF_STATE_BUSY); + wake_up(&itf->irq_queue); +} + +static int itf_wait_hw_ready(struct fimc_is_interface *itf) +{ + int t; + for (t = TRY_RECV_AWARE_COUNT; t >= 0; t--) { + unsigned int cfg = readl(itf->regs + INTMSR0); + if (INTMSR0_GET_INTMSD(0, cfg) == 0) +
[PATCH v12 07/12] [media] exynos5-fimc-is: Add sensor interface
Some sensors to be used with fimc-is are exclusively controlled by the fimc-is firmware. This minimal sensor driver provides the required info for the firmware to configure the sensors sitting on I2C bus. Signed-off-by: Arun Kumar K Reviewed-by: Sylwester Nawrocki --- drivers/media/platform/exynos5-is/fimc-is-sensor.c | 45 ++ drivers/media/platform/exynos5-is/fimc-is-sensor.h | 65 2 files changed, 110 insertions(+) create mode 100644 drivers/media/platform/exynos5-is/fimc-is-sensor.c create mode 100644 drivers/media/platform/exynos5-is/fimc-is-sensor.h diff --git a/drivers/media/platform/exynos5-is/fimc-is-sensor.c b/drivers/media/platform/exynos5-is/fimc-is-sensor.c new file mode 100644 index 000..475f1c3 --- /dev/null +++ b/drivers/media/platform/exynos5-is/fimc-is-sensor.c @@ -0,0 +1,45 @@ +/* + * Samsung EXYNOS5250 FIMC-IS (Imaging Subsystem) driver + * + * Copyright (C) 2013 Samsung Electronics Co., Ltd. + * Author: Arun Kumar K + * + * 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 "fimc-is-sensor.h" + +static const struct sensor_drv_data s5k6a3_drvdata = { + .id = FIMC_IS_SENSOR_ID_S5K6A3, + .open_timeout = S5K6A3_OPEN_TIMEOUT, + .setfile_name = "exynos5_s5k6a3_setfile.bin", +}; + +static const struct sensor_drv_data s5k4e5_drvdata = { + .id = FIMC_IS_SENSOR_ID_S5K4E5, + .open_timeout = S5K4E5_OPEN_TIMEOUT, + .setfile_name = "exynos5_s5k4e5_setfile.bin", +}; + +static const struct of_device_id fimc_is_sensor_of_ids[] = { + { + .compatible = "samsung,s5k6a3", + .data = &s5k6a3_drvdata, + }, + { + .compatible = "samsung,s5k4e5", + .data = &s5k4e5_drvdata, + }, + { } +}; + +const struct sensor_drv_data *exynos5_is_sensor_get_drvdata( + struct device_node *node) +{ + const struct of_device_id *of_id; + + of_id = of_match_node(fimc_is_sensor_of_ids, node); + return of_id ? of_id->data : NULL; +} diff --git a/drivers/media/platform/exynos5-is/fimc-is-sensor.h b/drivers/media/platform/exynos5-is/fimc-is-sensor.h new file mode 100644 index 000..0ba5733 --- /dev/null +++ b/drivers/media/platform/exynos5-is/fimc-is-sensor.h @@ -0,0 +1,65 @@ +/* + * Samsung EXYNOS4x12 FIMC-IS (Imaging Subsystem) driver + * + * Copyright (C) 2013 Samsung Electronics Co., Ltd. + * Author: Arun Kumar K + * + * 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. + */ +#ifndef FIMC_IS_SENSOR_H_ +#define FIMC_IS_SENSOR_H_ + +#include +#include + +#define S5K6A3_OPEN_TIMEOUT2000 /* ms */ +#define S5K6A3_SENSOR_WIDTH1392 +#define S5K6A3_SENSOR_HEIGHT 1392 + +#define S5K4E5_OPEN_TIMEOUT2000 /* ms */ +#define S5K4E5_SENSOR_WIDTH2560 +#define S5K4E5_SENSOR_HEIGHT 1920 + +#define SENSOR_WIDTH_PADDING 16 +#define SENSOR_HEIGHT_PADDING 10 + +enum fimc_is_sensor_id { + FIMC_IS_SENSOR_ID_S5K3H2 = 1, + FIMC_IS_SENSOR_ID_S5K6A3, + FIMC_IS_SENSOR_ID_S5K4E5, + FIMC_IS_SENSOR_ID_S5K3H7, + FIMC_IS_SENSOR_ID_CUSTOM, + FIMC_IS_SENSOR_ID_END +}; + +struct sensor_drv_data { + enum fimc_is_sensor_id id; + /* sensor open timeout in ms */ + unsigned short open_timeout; + char *setfile_name; +}; + +/** + * struct fimc_is_sensor - fimc-is sensor data structure + * @drvdata: a pointer to the sensor's parameters data structure + * @i2c_bus: ISP I2C bus index (0...1) + * @width: sensor active width + * @height: sensor active height + * @pixel_width: sensor effective pixel width (width + padding) + * @pixel_height: sensor effective pixel height (height + padding) + */ +struct fimc_is_sensor { + const struct sensor_drv_data *drvdata; + unsigned int i2c_bus; + unsigned int width; + unsigned int height; + unsigned int pixel_width; + unsigned int pixel_height; +}; + +const struct sensor_drv_data *exynos5_is_sensor_get_drvdata( + struct device_node *node); + +#endif /* FIMC_IS_SENSOR_H_ */ -- 1.7.9.5 -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v12 12/12] V4L: Add s5k4e5 sensor driver
This patch adds subdev driver for Samsung S5K4E5 raw image sensor. Like s5k6a3, it is also another fimc-is firmware controlled sensor. This minimal sensor driver doesn't do any I2C communications as its done by ISP firmware. It can be updated if needed to a regular sensor driver by adding the I2C communication. Signed-off-by: Arun Kumar K Reviewed-by: Sylwester Nawrocki --- drivers/media/i2c/Kconfig |8 ++ drivers/media/i2c/Makefile |1 + drivers/media/i2c/s5k4e5.c | 344 3 files changed, 353 insertions(+) create mode 100644 drivers/media/i2c/s5k4e5.c diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig index f7e9147..271028b 100644 --- a/drivers/media/i2c/Kconfig +++ b/drivers/media/i2c/Kconfig @@ -572,6 +572,14 @@ config VIDEO_S5K6A3 This is a V4L2 sensor-level driver for Samsung S5K6A3 raw camera sensor. +config VIDEO_S5K4E5 + tristate "Samsung S5K4E5 sensor support" + depends on MEDIA_CAMERA_SUPPORT + depends on I2C && VIDEO_V4L2 && VIDEO_V4L2_SUBDEV_API && OF + ---help--- + This is a V4L2 sensor-level driver for Samsung S5K4E5 raw + camera sensor. + config VIDEO_S5K4ECGX tristate "Samsung S5K4ECGX sensor support" depends on I2C && VIDEO_V4L2 && VIDEO_V4L2_SUBDEV_API diff --git a/drivers/media/i2c/Makefile b/drivers/media/i2c/Makefile index cf3cf03..0aeed8e 100644 --- a/drivers/media/i2c/Makefile +++ b/drivers/media/i2c/Makefile @@ -65,6 +65,7 @@ obj-$(CONFIG_VIDEO_SR030PC30) += sr030pc30.o obj-$(CONFIG_VIDEO_NOON010PC30)+= noon010pc30.o obj-$(CONFIG_VIDEO_S5K6AA) += s5k6aa.o obj-$(CONFIG_VIDEO_S5K6A3) += s5k6a3.o +obj-$(CONFIG_VIDEO_S5K4E5) += s5k4e5.o obj-$(CONFIG_VIDEO_S5K4ECGX) += s5k4ecgx.o obj-$(CONFIG_VIDEO_S5C73M3)+= s5c73m3/ obj-$(CONFIG_VIDEO_ADP1653)+= adp1653.o diff --git a/drivers/media/i2c/s5k4e5.c b/drivers/media/i2c/s5k4e5.c new file mode 100644 index 000..5d4007e --- /dev/null +++ b/drivers/media/i2c/s5k4e5.c @@ -0,0 +1,344 @@ +/* + * Samsung S5K4E5 image sensor driver + * + * Copyright (C) 2013 Samsung Electronics Co., Ltd. + * Author: Arun Kumar K + * + * 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 +#include +#include +#include +#include +#include + +#define S5K4E5_SENSOR_MAX_WIDTH2576 +#define S5K4E5_SENSOR_MAX_HEIGHT 1930 + +#define S5K4E5_SENSOR_MIN_WIDTH(32 + 16) +#define S5K4E5_SENSOR_MIN_HEIGHT (32 + 10) + +#define S5K4E5_DEF_WIDTH 1296 +#define S5K4E5_DEF_HEIGHT 732 + +#define S5K4E5_DRV_NAME"S5K4E5" +#define S5K4E5_CLK_NAME"extclk" + +#define S5K4E5_NUM_SUPPLIES2 + +#define S5K4E5_DEF_CLK_FREQ2400 + +/** + * struct s5k4e5 - s5k4e5 sensor data structure + * @dev: pointer to this I2C client device structure + * @subdev: the image sensor's v4l2 subdev + * @pad: subdev media source pad + * @supplies: image sensor's voltage regulator supplies + * @gpio_reset: GPIO connected to the sensor's reset pin + * @lock: mutex protecting the structure's members below + * @format: media bus format at the sensor's source pad + */ +struct s5k4e5 { + struct device *dev; + struct v4l2_subdev subdev; + struct media_pad pad; + struct regulator_bulk_data supplies[S5K4E5_NUM_SUPPLIES]; + int gpio_reset; + struct mutex lock; + struct v4l2_mbus_framefmt format; + struct clk *clock; + u32 clock_frequency; +}; + +static const char * const s5k4e5_supply_names[] = { + "svdda", + "svddio" +}; + +static inline struct s5k4e5 *sd_to_s5k4e5(struct v4l2_subdev *sd) +{ + return container_of(sd, struct s5k4e5, subdev); +} + +static const struct v4l2_mbus_framefmt s5k4e5_formats[] = { + { + .code = V4L2_MBUS_FMT_SGRBG10_1X10, + .colorspace = V4L2_COLORSPACE_SRGB, + .field = V4L2_FIELD_NONE, + } +}; + +static const struct v4l2_mbus_framefmt *find_sensor_format( + struct v4l2_mbus_framefmt *mf) +{ + int i; + + for (i = 0; i < ARRAY_SIZE(s5k4e5_formats); i++) + if (mf->code == s5k4e5_formats[i].code) + return &s5k4e5_formats[i]; + + return &s5k4e5_formats[0]; +} + +static int s5k4e5_enum_mbus_code(struct v4l2_subdev *sd, + struct v4l2_subdev_fh *fh, + struct v4l2_subdev_mbus_code_enum *code) +{ + if (code->index >= ARRAY_SIZE(s5k4e5_formats)) + return -EINVAL; + + code->code = s5k4e5_formats[code->index].code; + return 0; +} + +static void s5k4e
[PATCH v12 04/12] [media] exynos5-fimc-is: Add register definition and context header
This patch adds the register definition file for the fimc-is driver and also the header file containing the main context for the driver. Signed-off-by: Arun Kumar K Signed-off-by: Kilyeon Im Reviewed-by: Sylwester Nawrocki --- drivers/media/platform/exynos5-is/fimc-is-regs.h | 105 ++ drivers/media/platform/exynos5-is/fimc-is.h | 160 ++ 2 files changed, 265 insertions(+) create mode 100644 drivers/media/platform/exynos5-is/fimc-is-regs.h create mode 100644 drivers/media/platform/exynos5-is/fimc-is.h diff --git a/drivers/media/platform/exynos5-is/fimc-is-regs.h b/drivers/media/platform/exynos5-is/fimc-is-regs.h new file mode 100644 index 000..06aa466 --- /dev/null +++ b/drivers/media/platform/exynos5-is/fimc-is-regs.h @@ -0,0 +1,105 @@ +/* + * Samsung Exynos5 SoC series FIMC-IS driver + * + * Copyright (c) 2013 Samsung Electronics Co., Ltd + * Arun Kumar K + * Kil-yeon Lim + * + * 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. + */ + +#ifndef FIMC_IS_REGS_H +#define FIMC_IS_REGS_H + +/* WDT_ISP register */ +#define WDT0x0017 +/* MCUCTL register */ +#define MCUCTL 0x0018 +/* MCU Controller Register */ +#define MCUCTLR(MCUCTL+0x00) +#define MCUCTLR_AXI_ISPX_AWCACHE(x)((x) << 16) +#define MCUCTLR_AXI_ISPX_ARCACHE(x)((x) << 12) +#define MCUCTLR_MSWRST (1 << 0) +/* Boot Base OFfset Address Register */ +#define BBOAR (MCUCTL+0x04) +#define BBOAR_BBOA(x) ((x) << 0) + +/* Interrupt Generation Register 0 from Host CPU to VIC */ +#define INTGR0 (MCUCTL+0x08) +#define INTGR0_INTGC(n)(1 << ((n) + 16)) +#define INTGR0_INTGD(n)(1 << (n)) + +/* Interrupt Clear Register 0 from Host CPU to VIC */ +#define INTCR0 (MCUCTL+0x0c) +#define INTCR0_INTCC(n)(1 << ((n) + 16)) +#define INTCR0_INTCD(n)(1 << (n)) + +/* Interrupt Mask Register 0 from Host CPU to VIC */ +#define INTMR0 (MCUCTL+0x10) +#define INTMR0_INTMC(n)(1 << ((n) + 16)) +#define INTMR0_INTMD(n)(1 << (n)) + +/* Interrupt Status Register 0 from Host CPU to VIC */ +#define INTSR0 (MCUCTL+0x14) +#define INTSR0_GET_INTSD(n, x) (((x) >> (n)) & 0x1) +#define INTSR0_GET_INTSC(n, x) (((x) >> ((n) + 16)) & 0x1) + +/* Interrupt Mask Status Register 0 from Host CPU to VIC */ +#define INTMSR0(MCUCTL+0x18) +#define INTMSR0_GET_INTMSD(n, x) (((x) >> (n)) & 0x1) +#define INTMSR0_GET_INTMSC(n, x) (((x) >> ((n) + 16)) & 0x1) + +/* Interrupt Generation Register 1 from ISP CPU to Host IC */ +#define INTGR1 (MCUCTL+0x1c) +#define INTGR1_INTGC(n)(1 << (n)) + +/* Interrupt Clear Register 1 from ISP CPU to Host IC */ +#define INTCR1 (MCUCTL+0x20) +#define INTCR1_INTCC(n)(1 << (n)) + +/* Interrupt Mask Register 1 from ISP CPU to Host IC */ +#define INTMR1 (MCUCTL+0x24) +#define INTMR1_INTMC(n)(1 << (n)) + +/* Interrupt Status Register 1 from ISP CPU to Host IC */ +#define INTSR1 (MCUCTL+0x28) +/* Interrupt Mask Status Register 1 from ISP CPU to Host IC */ +#define INTMSR1(MCUCTL+0x2c) +/* Interrupt Clear Register 2 from ISP BLK's interrupts to Host IC */ +#define INTCR2 (MCUCTL+0x30) +#define INTCR2_INTCC(n)(1 << (n)) + +/* Interrupt Mask Register 2 from ISP BLK's interrupts to Host IC */ +#define INTMR2 (MCUCTL+0x34) +#define INTMR2_INTMCIS(n) (1 << (n)) + +/* Interrupt Status Register 2 from ISP BLK's interrupts to Host IC */ +#define INTSR2 (MCUCTL+0x38) +/* Interrupt Mask Status Register 2 from ISP BLK's interrupts to Host IC */ +#define INTMSR2(MCUCTL+0x3c) +/* General Purpose Output Control Register (0~17) */ +#define GPOCTLR(MCUCTL+0x40) +#define GPOCTLR_GPOG(n, x) ((x) << (n)) + +/* General Purpose Pad Output Enable Register (0~17) */ +#define GPOENCTLR (MCUCTL+0x44) +#define GPOENCTLR_GPOEN0(n, x) ((x) << (n)) + +/* General Purpose Input Control Register (0~17) */ +#define GPICTLR(MCUCTL+0x48) + +/* IS Shared Registers between ISP CPU and HOST CPU */ +#define ISSR(n)(MCUCTL + 0x80 + (n)) + +/* PMU for FIMC-IS*/ +#define PMUREG_CMU_RESET_ISP_SYS_PWR_REG 0x1584 +#define PMUREG_ISP_ARM_CONFIGURA
[PATCH v12 05/12] [media] exynos5-fimc-is: Add isp subdev
fimc-is driver takes video data input from the ISP video node which is added in this patch. This node accepts Bayer input buffers which is given from the IS sensors. Signed-off-by: Arun Kumar K Signed-off-by: Kilyeon Im Reviewed-by: Sylwester Nawrocki --- drivers/media/platform/exynos5-is/fimc-is-isp.c | 534 +++ drivers/media/platform/exynos5-is/fimc-is-isp.h | 90 2 files changed, 624 insertions(+) create mode 100644 drivers/media/platform/exynos5-is/fimc-is-isp.c create mode 100644 drivers/media/platform/exynos5-is/fimc-is-isp.h diff --git a/drivers/media/platform/exynos5-is/fimc-is-isp.c b/drivers/media/platform/exynos5-is/fimc-is-isp.c new file mode 100644 index 000..7bd603f --- /dev/null +++ b/drivers/media/platform/exynos5-is/fimc-is-isp.c @@ -0,0 +1,534 @@ +/* + * Samsung EXYNOS5250 FIMC-IS (Imaging Subsystem) driver + * + * Copyright (C) 2013 Samsung Electronics Co., Ltd. + * Arun Kumar K + * + * 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 "fimc-is.h" + +#define ISP_DRV_NAME "fimc-is-isp" + +static const struct fimc_is_fmt formats[] = { + { + .name = "Bayer GR-BG 8bits", + .fourcc = V4L2_PIX_FMT_SGRBG8, + .depth = { 8 }, + .num_planes = 1, + }, + { + .name = "Bayer GR-BG 10bits", + .fourcc = V4L2_PIX_FMT_SGRBG10, + .depth = { 16 }, + .num_planes = 1, + }, + { + .name = "Bayer GR-BG 12bits", + .fourcc = V4L2_PIX_FMT_SGRBG12, + .depth = { 16 }, + .num_planes = 1, + }, +}; +#define NUM_FORMATS ARRAY_SIZE(formats) + +static const struct fimc_is_fmt *find_format(struct v4l2_format *f) +{ + unsigned int i; + + for (i = 0; i < NUM_FORMATS; i++) + if (formats[i].fourcc == f->fmt.pix_mp.pixelformat) + return &formats[i]; + return NULL; +} + +static int isp_video_output_start_streaming(struct vb2_queue *vq, + unsigned int count) +{ + struct fimc_is_isp *isp = vb2_get_drv_priv(vq); + + set_bit(STATE_RUNNING, &isp->output_state); + return 0; +} + +static int isp_video_output_stop_streaming(struct vb2_queue *vq) +{ + struct fimc_is_isp *isp = vb2_get_drv_priv(vq); + struct fimc_is_buf *buf; + + /* Release unused buffers */ + while (!list_empty(&isp->wait_queue)) { + buf = fimc_is_isp_wait_queue_get(isp); + vb2_buffer_done(&buf->vb, VB2_BUF_STATE_ERROR); + } + while (!list_empty(&isp->run_queue)) { + buf = fimc_is_isp_run_queue_get(isp); + vb2_buffer_done(&buf->vb, VB2_BUF_STATE_ERROR); + } + + clear_bit(STATE_RUNNING, &isp->output_state); + return 0; +} + +static int isp_video_output_queue_setup(struct vb2_queue *vq, + const struct v4l2_format *pfmt, + unsigned int *num_buffers, unsigned int *num_planes, + unsigned int sizes[], void *allocators[]) +{ + struct fimc_is_isp *isp = vb2_get_drv_priv(vq); + const struct fimc_is_fmt *fmt = isp->fmt; + unsigned int wh, i; + + if (!fmt) + return -EINVAL; + + *num_planes = fmt->num_planes; + wh = isp->width * isp->height; + + for (i = 0; i < *num_planes; i++) { + allocators[i] = isp->alloc_ctx; + sizes[i] = (wh * fmt->depth[i]) / 8; + } + return 0; +} + +static int isp_video_output_buffer_init(struct vb2_buffer *vb) +{ + struct fimc_is_buf *buf = container_of(vb, struct fimc_is_buf, vb); + + buf->paddr[0] = vb2_dma_contig_plane_dma_addr(vb, 0); + return 0; +} + +static int isp_video_output_buffer_prepare(struct vb2_buffer *vb) +{ + struct vb2_queue *vq = vb->vb2_queue; + struct fimc_is_isp *isp = vb2_get_drv_priv(vq); + unsigned long size; + + size = (isp->width * isp->height * isp->fmt->depth[0]) / 8; + if (vb2_plane_size(vb, 0) < size) { + v4l2_err(&isp->subdev, "User buffer too small (%ld < %ld)\n", +vb2_plane_size(vb, 0), size); + return -EINVAL; + } + vb2_set_plane_payload(vb, 0, size); + + return 0; +} + +static void isp_video_output_buffer_queue(struct vb2_buffer *vb) +{ + struct vb2_queue *vq = vb->vb2_queue; + struct fimc_is_isp *isp = vb2_get_drv_priv(vq); + struct fimc_is_buf *buf = container_of(vb, struct fimc_is_buf, vb); + + fimc_is_pipeline_buf_lock(isp->pipeline); + fimc_is_isp_wait_queue_add(isp, buf); + fimc_is_pi
[PATCH v12 06/12] [media] exynos5-fimc-is: Add scaler subdev
FIMC-IS has two hardware scalers named as scaler-codec and scaler-preview. This patch adds the common code handling the video nodes and subdevs of both the scalers. Signed-off-by: Arun Kumar K Signed-off-by: Kilyeon Im Reviewed-by: Sylwester Nawrocki --- drivers/media/platform/exynos5-is/fimc-is-scaler.c | 476 drivers/media/platform/exynos5-is/fimc-is-scaler.h | 106 + 2 files changed, 582 insertions(+) create mode 100644 drivers/media/platform/exynos5-is/fimc-is-scaler.c create mode 100644 drivers/media/platform/exynos5-is/fimc-is-scaler.h diff --git a/drivers/media/platform/exynos5-is/fimc-is-scaler.c b/drivers/media/platform/exynos5-is/fimc-is-scaler.c new file mode 100644 index 000..029eb8b --- /dev/null +++ b/drivers/media/platform/exynos5-is/fimc-is-scaler.c @@ -0,0 +1,476 @@ +/* + * Samsung EXYNOS5250 FIMC-IS (Imaging Subsystem) driver + * + * Copyright (C) 2013 Samsung Electronics Co., Ltd. + * Arun Kumar K + * + * 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 "fimc-is.h" + +#define IS_SCALER_DRV_NAME "fimc-is-scaler" + +static const struct fimc_is_fmt formats[] = { + { + .name = "YUV 4:2:0 3p MultiPlanar", + .fourcc = V4L2_PIX_FMT_YUV420M, + .depth = {8, 2, 2}, + .num_planes = 3, + }, + { + .name = "YUV 4:2:0 2p MultiPlanar", + .fourcc = V4L2_PIX_FMT_NV12M, + .depth = {8, 4}, + .num_planes = 2, + }, + { + .name = "YUV 4:2:2 1p MultiPlanar", + .fourcc = V4L2_PIX_FMT_NV16, + .depth = {16}, + .num_planes = 1, + }, +}; +#define NUM_FORMATS ARRAY_SIZE(formats) + +static const struct fimc_is_fmt *find_format(struct v4l2_format *f) +{ + unsigned int i; + + for (i = 0; i < NUM_FORMATS; i++) { + if (formats[i].fourcc == f->fmt.pix_mp.pixelformat) + return &formats[i]; + } + return NULL; +} + +static int scaler_video_capture_start_streaming(struct vb2_queue *vq, + unsigned int count) +{ + struct fimc_is_scaler *ctx = vb2_get_drv_priv(vq); + int ret; + + ret = fimc_is_pipeline_scaler_start(ctx->pipeline, + ctx->scaler_id, + vq->num_buffers, + ctx->fmt->num_planes); + if (ret) { + v4l2_err(&ctx->subdev, "Scaler start failed.\n"); + return -EINVAL; + } + + set_bit(STATE_RUNNING, &ctx->capture_state); + return 0; +} + +static int scaler_video_capture_stop_streaming(struct vb2_queue *vq) +{ + struct fimc_is_scaler *ctx = vb2_get_drv_priv(vq); + struct fimc_is_buf *buf; + int ret; + + ret = fimc_is_pipeline_scaler_stop(ctx->pipeline, ctx->scaler_id); + if (ret) + v4l2_info(&ctx->subdev, "Scaler already stopped.\n"); + + /* Release un-used buffers */ + while (!list_empty(&ctx->wait_queue)) { + buf = fimc_is_scaler_wait_queue_get(ctx); + vb2_buffer_done(&buf->vb, VB2_BUF_STATE_ERROR); + } + while (!list_empty(&ctx->run_queue)) { + buf = fimc_is_scaler_run_queue_get(ctx); + vb2_buffer_done(&buf->vb, VB2_BUF_STATE_ERROR); + } + + clear_bit(STATE_RUNNING, &ctx->capture_state); + return 0; +} + +static int scaler_video_capture_queue_setup(struct vb2_queue *vq, + const struct v4l2_format *pfmt, + unsigned int *num_buffers, unsigned int *num_planes, + unsigned int sizes[], void *allocators[]) +{ + struct fimc_is_scaler *ctx = vb2_get_drv_priv(vq); + const struct fimc_is_fmt *fmt = ctx->fmt; + unsigned int wh; + int i; + + if (!fmt) + return -EINVAL; + + *num_planes = fmt->num_planes; + wh = ctx->width * ctx->height; + + for (i = 0; i < *num_planes; i++) { + allocators[i] = ctx->alloc_ctx; + sizes[i] = (wh * fmt->depth[i]) / 8; + } + return 0; +} + +static int scaler_video_capture_buffer_init(struct vb2_buffer *vb) +{ + struct vb2_queue *vq = vb->vb2_queue; + struct fimc_is_scaler *ctx = vb2_get_drv_priv(vq); + struct fimc_is_buf *buf = container_of(vb, struct fimc_is_buf, vb); + const struct fimc_is_fmt *fmt; + int i; + + fmt = ctx->fmt; + for (i = 0; i < fmt->num_planes; i++) + buf->paddr[i] = vb2_dma_contig_plane_dma_addr(vb, i); + + return 0; +} + +static int scaler_video_capture_buffer_prepare(struct vb2_buffer
[PATCH v12 01/12] [media] exynos5-fimc-is: Add Exynos5 FIMC-IS device tree bindings documentation
The patch adds the DT binding documentation for Samsung Exynos5 SoC series imaging subsystem (FIMC-IS). Signed-off-by: Arun Kumar K Reviewed-by: Sylwester Nawrocki Acked-by: Mark Rutland --- .../devicetree/bindings/media/exynos5-fimc-is.txt | 113 1 file changed, 113 insertions(+) create mode 100644 Documentation/devicetree/bindings/media/exynos5-fimc-is.txt diff --git a/Documentation/devicetree/bindings/media/exynos5-fimc-is.txt b/Documentation/devicetree/bindings/media/exynos5-fimc-is.txt new file mode 100644 index 000..658d4a9 --- /dev/null +++ b/Documentation/devicetree/bindings/media/exynos5-fimc-is.txt @@ -0,0 +1,113 @@ +Samsung EXYNOS5 SoC series Imaging Subsystem (FIMC-IS) +-- + +The camera subsystem on Samsung Exynos5 SoC has some changes relative +to previous SoC versions. Exynos5 has almost similar MIPI-CSIS and +FIMC-LITE IPs but has a much improved version of FIMC-IS which can +handle sensor controls and camera post-processing operations. The +Exynos5 FIMC-IS has a dedicated ARM Cortex A5 processor, many +post-processing blocks (ISP, DRC, FD, ODC, DIS, 3DNR) and two +dedicated scalers (SCC and SCP). + +fimc-is node + + +Required properties: + +- compatible: should contain "samsung,exynos5250-fimc-is" +- reg : physical base address and size of the memory mapped + registers +- interrupts: interrupt-specifier for the fimc-is interrupt +- clocks: list of clock specifiers, corresponding to entries in + clock-names property +- clock-names : must contain "isp", "mcu_isp", "isp_div0", "isp_div1", + "isp_divmpwm", "mcu_isp_div0", "mcu_isp_div1" entries, + matching entries in the clocks property +- samsung,pmu : phandle to the Power Management Unit (PMU) node + +i2c-isp (ISP I2C bus controller) nodes +-- +The i2c-isp nodes should be children of the fimc-is node. + +Required properties: + +- compatible : must contain "samsung,exynos4212-i2c-isp" for Exynos4212, + Exynos4412 and Exynos5250 SoCs +- reg : physical base address and length of the registers set +- clocks : should contain gate clock specifier for this controller +- clock-names : should contain "i2c_isp" for the gate clock +- pinctrl-0: phandle of the pinctrl node for the i2c isp +- pinctrl-names : must contain "default" + +ranges, #address-cells, and #size-cells should be present as appropriate. + +Device tree nodes of the image sensors controlled directly by the FIMC-IS +firmware must be child nodes of their corresponding ISP I2C bus controller node. +The data link of these image sensors must be specified using the common video +interfaces bindings, defined in video-interfaces.txt. + +Example: + + fimc_is: fimc-is@1300 { + compatible = "samsung,exynos5250-fimc-is"; + #address-cells = <1>; + #size-cells = <1>; + ranges; + reg = <0x1300 0x20>; + interrupt-parent = <&combiner>; + interrupts = <19 1>; + clocks = <&clock 346>, <&clock 347>, <&clock 512>, + <&clock 513>, <&clock 514>, <&clock 515>, + <&clock 516>; + clock-names = "isp", "mcu_isp", "isp_div0", "isp_div1", + "isp_divmpwm", "mcu_isp_div0", + "mcu_isp_div1"; + samsung,pmu = <&pmu>; + + i2c0_isp: i2c-isp@1313 { + compatible = "samsung,exynos4212-i2c-isp"; + reg = <0x1313 0x100>; + #address-cells = <1>; + #size-cells = <0>; + clocks = <&clock 352>; + clock-names = "i2c_isp"; + pinctrl-0 = <&cam_i2c0_bus>; + pinctrl-names = "default"; + }; + + i2c1_isp: i2c-isp@1314 { + compatible = "samsung,exynos4212-i2c-isp"; + reg = <0x1314 0x100>; + #address-cells = <1>; + #size-cells = <0>; + clocks = <&clock 353>; + clock-names = "i2c_isp"; + pinctrl-0 = <&cam_i2c1_bus>; + pinctrl-names = "default"; + }; + }; + +In the board specific file the sensor nodes can be provided. For the sensor +node documentation for s5k4e5, please refer to samsung-s5k4e5.txt + + fimc-is@1300 { + status = "okay"; + + i2c-isp@1313 { + s5k4e5@20 { + compatible = "samsung,s5k4e5"; + reg = <0x20>; +
[PATCH v12 02/12] [media] exynos5-fimc-is: Add driver core files
This driver is for the FIMC-IS IP available in Samsung Exynos5 SoC onwards. This patch adds the core files for the new driver. Signed-off-by: Arun Kumar K Signed-off-by: Kilyeon Im Reviewed-by: Sylwester Nawrocki --- drivers/media/platform/exynos5-is/fimc-is-core.c | 386 ++ drivers/media/platform/exynos5-is/fimc-is-core.h | 117 +++ 2 files changed, 503 insertions(+) create mode 100644 drivers/media/platform/exynos5-is/fimc-is-core.c create mode 100644 drivers/media/platform/exynos5-is/fimc-is-core.h diff --git a/drivers/media/platform/exynos5-is/fimc-is-core.c b/drivers/media/platform/exynos5-is/fimc-is-core.c new file mode 100644 index 000..136e3c1 --- /dev/null +++ b/drivers/media/platform/exynos5-is/fimc-is-core.c @@ -0,0 +1,386 @@ +/* + * Samsung EXYNOS5 FIMC-IS (Imaging Subsystem) driver +* + * Copyright (C) 2013 Samsung Electronics Co., Ltd. + * Arun Kumar K + * + * 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 + +#include "fimc-is.h" +#include "fimc-is-i2c.h" + +#define CLK_MCU_ISP_DIV0_FREQ (200 * 100) +#define CLK_MCU_ISP_DIV1_FREQ (100 * 100) +#define CLK_ISP_DIV0_FREQ (134 * 100) +#define CLK_ISP_DIV1_FREQ (68 * 100) +#define CLK_ISP_DIVMPWM_FREQ (34 * 100) + +static const char * const fimc_is_clock_name[] = { + [IS_CLK_ISP]= "isp", + [IS_CLK_MCU_ISP]= "mcu_isp", + [IS_CLK_ISP_DIV0] = "isp_div0", + [IS_CLK_ISP_DIV1] = "isp_div1", + [IS_CLK_ISP_DIVMPWM]= "isp_divmpwm", + [IS_CLK_MCU_ISP_DIV0] = "mcu_isp_div0", + [IS_CLK_MCU_ISP_DIV1] = "mcu_isp_div1", +}; + +static void fimc_is_put_clocks(struct fimc_is *is) +{ + int i; + + for (i = 0; i < IS_CLK_MAX_NUM; i++) { + if (IS_ERR(is->clock[i])) + continue; + clk_unprepare(is->clock[i]); + clk_put(is->clock[i]); + is->clock[i] = ERR_PTR(-EINVAL); + } +} + +static int fimc_is_get_clocks(struct fimc_is *is) +{ + struct device *dev = &is->pdev->dev; + int i, ret; + + for (i = 0; i < IS_CLK_MAX_NUM; i++) { + is->clock[i] = clk_get(dev, fimc_is_clock_name[i]); + if (IS_ERR(is->clock[i])) + goto err; + ret = clk_prepare(is->clock[i]); + if (ret < 0) { + clk_put(is->clock[i]); + is->clock[i] = ERR_PTR(-EINVAL); + goto err; + } + } + return 0; +err: + fimc_is_put_clocks(is); + dev_err(dev, "Failed to get clock: %s\n", fimc_is_clock_name[i]); + return -ENXIO; +} + +static int fimc_is_configure_clocks(struct fimc_is *is) +{ + int i, ret; + + for (i = 0; i < IS_CLK_MAX_NUM; i++) + is->clock[i] = ERR_PTR(-EINVAL); + + ret = fimc_is_get_clocks(is); + if (ret) + return ret; + + /* Set rates */ + ret = clk_set_rate(is->clock[IS_CLK_MCU_ISP_DIV0], + CLK_MCU_ISP_DIV0_FREQ); + if (ret) + return ret; + ret = clk_set_rate(is->clock[IS_CLK_MCU_ISP_DIV1], + CLK_MCU_ISP_DIV1_FREQ); + if (ret) + return ret; + ret = clk_set_rate(is->clock[IS_CLK_ISP_DIV0], CLK_ISP_DIV0_FREQ); + if (ret) + return ret; + ret = clk_set_rate(is->clock[IS_CLK_ISP_DIV1], CLK_ISP_DIV1_FREQ); + if (ret) + return ret; + return clk_set_rate(is->clock[IS_CLK_ISP_DIVMPWM], + CLK_ISP_DIVMPWM_FREQ); +} + +static void fimc_is_pipelines_destroy(struct fimc_is *is) +{ + int i; + + for (i = 0; i < is->drvdata->num_instances; i++) + fimc_is_pipeline_destroy(&is->pipeline[i]); +} + +static int fimc_is_parse_sensor_config(struct fimc_is *is, unsigned int index, + struct device_node *node) +{ + struct fimc_is_sensor *sensor = &is->sensor[index]; + u32 tmp = 0; + int ret; + + sensor->drvdata = exynos5_is_sensor_get_drvdata(node); + if (!sensor->drvdata) { + dev_err(&is->pdev->dev, "no driver data found for: %s\n", +node->full_name); + return -EINVAL; + } + + node = v4l2_of_get_next_endpoint(node, NULL); + if (!node) + return -ENXIO; + + node = v4l2_of_get_remote_port(node); + if (!node) + return -ENXIO; + + /* Use MIPI-CSIS channel id to determine the ISP I2C bus index. */ + ret = of_property_read_u32(node, "reg", &tmp); + i
[PATCH v12 00/12] Exynos5 IS driver
The patch series adds support for exynos5 fimc-is driver and a new sensor s5k4e5. The media driver part is omitted form this series as it is already applied. Changes from v11 - Addressed review comments from Sakari Ailus http://www.mail-archive.com/linux-media@vger.kernel.org/msg68134.html Changes from v10 --- - Addressed DT binding review comments from Mark Rutland https://www.mail-archive.com/linux-media@vger.kernel.org/msg67806.html https://www.mail-archive.com/linux-media@vger.kernel.org/msg67808.html Changes from v9 --- - Addressed review comments from Hans and Sylwester http://www.mail-archive.com/linux-media@vger.kernel.org/msg67102.html http://www.mail-archive.com/linux-media@vger.kernel.org/msg67624.html http://www.mail-archive.com/linux-media@vger.kernel.org/msg67623.html - Skipped already applied media driver Changes from v8 --- - Moved i2c-isp device nodes into the fimc-is node as suggested by Sylwester - Addressed comments given by Sylwester and Philipp Zabel Changes from v7 --- - Addressed few DT related review comments from Sylwester http://www.mail-archive.com/linux-media@vger.kernel.org/msg66403.html - Few fixes added after some regression testing Changes from v6 --- - Addressed DT binding doc review comments from Sylwester http://www.mail-archive.com/linux-media@vger.kernel.org/msg65771.html http://www.mail-archive.com/linux-media@vger.kernel.org/msg65772.html Changes from v5 --- - Addressed review comments from Sylwester http://www.mail-archive.com/linux-media@vger.kernel.org/msg65578.html http://www.mail-archive.com/linux-media@vger.kernel.org/msg65605.html Changes from v4 --- - Addressed all review comments from Sylwester - Added separate PMU node as suggested by Stephen Warren - Added phandle based discovery of subdevs instead of node name Changes from v3 --- - Dropped the RFC tag - Addressed all review comments from Sylwester and Sachin - Removed clock provider for media dev - Added s5k4e5 sensor devicetree binding doc Changes from v2 --- - Added exynos5 media device driver from Shaik to this series - Added ISP pipeline support in media device driver - Based on Sylwester's latest exynos4-is development - Asynchronos registration of sensor subdevs - Made independent IS-sensor support - Add s5k4e5 sensor driver - Addressed review comments from Sylwester, Hans, Andrzej, Sachin Changes from v1 --- - Addressed all review comments from Sylwester - Made sensor subdevs as independent i2c devices - Lots of cleanup - Debugfs support added - Removed PMU global register access Arun Kumar K (12): [media] exynos5-fimc-is: Add Exynos5 FIMC-IS device tree bindings documentation [media] exynos5-fimc-is: Add driver core files [media] exynos5-fimc-is: Add common driver header files [media] exynos5-fimc-is: Add register definition and context header [media] exynos5-fimc-is: Add isp subdev [media] exynos5-fimc-is: Add scaler subdev [media] exynos5-fimc-is: Add sensor interface [media] exynos5-fimc-is: Add the hardware pipeline control [media] exynos5-fimc-is: Add the hardware interface module [media] exynos5-is: Add Kconfig and Makefile V4L: Add DT binding doc for s5k4e5 image sensor V4L: Add s5k4e5 sensor driver .../devicetree/bindings/media/exynos5-fimc-is.txt | 113 ++ .../devicetree/bindings/media/samsung-s5k4e5.txt | 45 + drivers/media/i2c/Kconfig |8 + drivers/media/i2c/Makefile |1 + drivers/media/i2c/s5k4e5.c | 344 drivers/media/platform/Kconfig |1 + drivers/media/platform/Makefile|1 + drivers/media/platform/exynos5-is/Kconfig | 20 + drivers/media/platform/exynos5-is/Makefile |7 + drivers/media/platform/exynos5-is/fimc-is-cmd.h| 187 +++ drivers/media/platform/exynos5-is/fimc-is-core.c | 387 + drivers/media/platform/exynos5-is/fimc-is-core.h | 117 ++ drivers/media/platform/exynos5-is/fimc-is-err.h| 257 +++ .../media/platform/exynos5-is/fimc-is-interface.c | 810 ++ .../media/platform/exynos5-is/fimc-is-interface.h | 124 ++ drivers/media/platform/exynos5-is/fimc-is-isp.c| 534 ++ drivers/media/platform/exynos5-is/fimc-is-isp.h| 90 ++ .../media/platform/exynos5-is/fimc-is-metadata.h | 767 + drivers/media/platform/exynos5-is/fimc-is-param.h | 1159 + .../media/platform/exynos5-is/fimc-is-pipeline.c | 1699 .../media/platform/exynos5-is/fimc-is-pipeline.h | 129 ++ drivers/media/platform/exynos5-is/fimc-is-regs.h | 105 ++ drivers/media/platform/exynos5-is/fimc-is-scaler.c | 476 ++ drivers/media/platform/exynos5-is/fimc-is-scaler.h | 106 ++ drivers/media/platform/exynos5-is/fimc-is-sensor.c | 45 + drivers/media/platform/exynos5-is
Re: [PATCH] [media] az6007: support Technisat Cablestar Combo HDCI (minus remote)
Quote/Cytat - "Janusz S. Bien" (Tue 05 Nov 2013 07:14:22 AM CET): Thank you very much for the patch. Quote/Cytat - Roland Scheidegger (Sat 02 Nov 2013 08:49:32 PM CET): [...] Originally based on idea found on http://www.linuxtv.org/wiki/index.php/TechniSat_CableStar_Combo_HD_CI claiming only id needs to be added (but failed to mention it only worked because the driver couldn't find the h7 drx-k firmware...). Together with Tobias Wessel, another user of the device, we have updated and extended the wiki entry. The problem is that although I use the same system as Tobias (Debian wheezy) and it seems we installed media_build in the identical way, it works OK for Tobias but not for me, My problem was solved by Tobias - thank you very much! As could be expected, I've installed the software differently... I have erroneusly assumed that a patch posted on the list is immediately included http://git.linuxtv.org/media_build.git. I see it now at https://patchwork.linuxtv.org/patch/20558/ together with my previous misleading letter... Regards Janusz -- Prof. dr hab. Janusz S. Bień - Uniwersytet Warszawski (Katedra Lingwistyki Formalnej) Prof. Janusz S. Bień - University of Warsaw (Formal Linguistics Department) jsb...@uw.edu.pl, jsb...@mimuw.edu.pl, http://fleksem.klf.uw.edu.pl/~jsbien/ -- 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: [PATCHv2 22/29] v4l2-async: Don't use dynamic static allocation
Em Tue, 05 Nov 2013 12:42:23 +0100 Sylwester Nawrocki escreveu: > On 05/11/13 12:36, Mauro Carvalho Chehab wrote: > >>> diff --git a/drivers/media/v4l2-core/v4l2-async.c > >>> b/drivers/media/v4l2-core/v4l2-async.c > >>> > > index c85d69da35bd..071596869036 100644 > >>> > > --- a/drivers/media/v4l2-core/v4l2-async.c > >>> > > +++ b/drivers/media/v4l2-core/v4l2-async.c > >>> > > @@ -189,12 +189,14 @@ void v4l2_async_notifier_unregister(struct > >>> > > v4l2_async_notifier *notifier) > >>> > > struct v4l2_subdev *sd, *tmp; > >>> > > unsigned int notif_n_subdev = notifier->num_subdevs; > >>> > > unsigned int n_subdev = min(notif_n_subdev, V4L2_MAX_SUBDEVS); > >>> > > - struct device *dev[n_subdev]; > >>> > > + struct device **dev; > >>> > > int i = 0; > >>> > > > >>> > > if (!notifier->v4l2_dev) > >>> > > return; > >>> > > > >>> > > + dev = kmalloc(sizeof(*dev) * n_subdev, GFP_KERNEL); > >>> > > + > >> > > >> > No check for dev == NULL? > > Well, what should be done in this case? > > > > We could do the changes below: > > > > void v4l2_async_notifier_unregister(struct v4l2_async_notifier *notifier) > > { > > struct v4l2_subdev *sd, *tmp; > > unsigned int notif_n_subdev = notifier->num_subdevs; > > unsigned int n_subdev = min(notif_n_subdev, V4L2_MAX_SUBDEVS); > > - struct device *dev[n_subdev]; > > + struct device **dev; > > int i = 0; > > > > if (!notifier->v4l2_dev) > > return; > > > > + dev = kmalloc(sizeof(*dev) * n_subdev, GFP_KERNEL); > > + if (!dev) { > > + WARN_ON(true); > > + return; > > + } > > + > > mutex_lock(&list_lock); > > > > list_del(¬ifier->list); > > > > list_for_each_entry_safe(sd, tmp, ¬ifier->done, async_list) { > > dev[i] = get_device(sd->dev); > > > > v4l2_async_cleanup(sd); > > > > /* If we handled USB devices, we'd have to lock the parent > > too */ > > device_release_driver(dev[i++]); > > > > if (notifier->unbind) > > notifier->unbind(notifier, sd, sd->asd); > > } > > > > mutex_unlock(&list_lock); > > > > while (i--) { > > struct device *d = dev[i]; > > > > if (d && device_attach(d) < 0) { > > const char *name = "(none)"; > > int lock = device_trylock(d); > > > > if (lock && d->driver) > > name = d->driver->name; > > dev_err(d, "Failed to re-probe to %s\n", name); > > if (lock) > > device_unlock(d); > > } > > put_device(d); > > } > > + kfree(dev); > > > > notifier->v4l2_dev = NULL; > > > > /* > > * Don't care about the waiting list, it is initialised and > > populated > > * upon notifier registration. > > */ > > } > > EXPORT_SYMBOL(v4l2_async_notifier_unregister); > > > > But I suspect that this will cause an OOPS anyway, as the device will be > > only half-removed. So, it would likely OOPS at device removal or if the > > device got probed again, at probing time. > > > > So, IMHO, we should have at least a WARN_ON() for this case. > > > > Do you have a better idea? > > This is how Guennadi's patch looked like when it used dynamic allocation: > > http://www.spinics.net/lists/linux-sh/msg18194.html Thanks for the tip! The following patch should do the trick (generated with -U10, in order to show the entire function): [PATCHv3] v4l2-async: Don't use dynamic static allocation Dynamic static allocation is evil, as Kernel stack is too low, and compilation complains about it on some archs: drivers/media/v4l2-core/v4l2-async.c:238:1: warning: 'v4l2_async_notifier_unregister' uses dynamic stack allocation [enabled by default] Instead, let's enforce a limit for the buffer. In this specific case, there's a hard limit imposed by V4L2_MAX_SUBDEVS, with is currently 128. That means that the buffer size can be up to 128x8 = 1024 bytes (on a 64bits kernel), with is too big for stack. Worse than that, someone could increase it and cause real troubles. So, let's use dynamically allocated data, instead. Signed-off-by: Mauro Carvalho Chehab Cc: Guennadi Liakhovetski diff --git a/drivers/media/v4l2-core/v4l2-async.c b/drivers/media/v4l2-core/v4l2-async.c index c85d69da35bd..b56c9f300ecb 100644 --- a/drivers/media/v4l2-core/v4l2-async.c +++ b/drivers/media/v4l2-core/v4l2-async.c @@ -182,59 +182,84 @@ int v4l2_async_notifier_register(struct v4l2_device *v4l2_dev, return 0; } EXPORT_SYMBOL(v4l2_async_notifier_register); void v4l2_async_notifier_unregister(struct v4l2_async_notifier *notifier) { s
Re: [PATCHv2 22/29] v4l2-async: Don't use dynamic static allocation
On 05/11/13 12:36, Mauro Carvalho Chehab wrote: >>> diff --git a/drivers/media/v4l2-core/v4l2-async.c >>> b/drivers/media/v4l2-core/v4l2-async.c >>> > > index c85d69da35bd..071596869036 100644 >>> > > --- a/drivers/media/v4l2-core/v4l2-async.c >>> > > +++ b/drivers/media/v4l2-core/v4l2-async.c >>> > > @@ -189,12 +189,14 @@ void v4l2_async_notifier_unregister(struct >>> > > v4l2_async_notifier *notifier) >>> > > struct v4l2_subdev *sd, *tmp; >>> > > unsigned int notif_n_subdev = notifier->num_subdevs; >>> > > unsigned int n_subdev = min(notif_n_subdev, V4L2_MAX_SUBDEVS); >>> > > - struct device *dev[n_subdev]; >>> > > + struct device **dev; >>> > > int i = 0; >>> > > >>> > > if (!notifier->v4l2_dev) >>> > > return; >>> > > >>> > > + dev = kmalloc(sizeof(*dev) * n_subdev, GFP_KERNEL); >>> > > + >> > >> > No check for dev == NULL? > Well, what should be done in this case? > > We could do the changes below: > > void v4l2_async_notifier_unregister(struct v4l2_async_notifier *notifier) > { > struct v4l2_subdev *sd, *tmp; > unsigned int notif_n_subdev = notifier->num_subdevs; > unsigned int n_subdev = min(notif_n_subdev, V4L2_MAX_SUBDEVS); > - struct device *dev[n_subdev]; > + struct device **dev; > int i = 0; > > if (!notifier->v4l2_dev) > return; > > + dev = kmalloc(sizeof(*dev) * n_subdev, GFP_KERNEL); > + if (!dev) { > + WARN_ON(true); > + return; > + } > + > mutex_lock(&list_lock); > > list_del(¬ifier->list); > > list_for_each_entry_safe(sd, tmp, ¬ifier->done, async_list) { > dev[i] = get_device(sd->dev); > > v4l2_async_cleanup(sd); > > /* If we handled USB devices, we'd have to lock the parent > too */ > device_release_driver(dev[i++]); > > if (notifier->unbind) > notifier->unbind(notifier, sd, sd->asd); > } > > mutex_unlock(&list_lock); > > while (i--) { > struct device *d = dev[i]; > > if (d && device_attach(d) < 0) { > const char *name = "(none)"; > int lock = device_trylock(d); > > if (lock && d->driver) > name = d->driver->name; > dev_err(d, "Failed to re-probe to %s\n", name); > if (lock) > device_unlock(d); > } > put_device(d); > } > + kfree(dev); > > notifier->v4l2_dev = NULL; > > /* > * Don't care about the waiting list, it is initialised and populated > * upon notifier registration. > */ > } > EXPORT_SYMBOL(v4l2_async_notifier_unregister); > > But I suspect that this will cause an OOPS anyway, as the device will be > only half-removed. So, it would likely OOPS at device removal or if the > device got probed again, at probing time. > > So, IMHO, we should have at least a WARN_ON() for this case. > > Do you have a better idea? This is how Guennadi's patch looked like when it used dynamic allocation: http://www.spinics.net/lists/linux-sh/msg18194.html -- 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: [PATCHv2 22/29] v4l2-async: Don't use dynamic static allocation
Em Mon, 04 Nov 2013 14:15:04 +0100 Hans Verkuil escreveu: > On 11/02/2013 02:31 PM, Mauro Carvalho Chehab wrote: > > Dynamic static allocation is evil, as Kernel stack is too low, and > > compilation complains about it on some archs: > > > > drivers/media/v4l2-core/v4l2-async.c:238:1: warning: > > 'v4l2_async_notifier_unregister' uses dynamic stack allocation [enabled by > > default] > > > > Instead, let's enforce a limit for the buffer. > > > > In this specific case, there's a hard limit imposed by V4L2_MAX_SUBDEVS, > > with is currently 128. That means that the buffer size can be up to > > 128x8 = 1024 bytes (on a 64bits kernel), with is too big for stack. > > > > Worse than that, someone could increase it and cause real troubles. > > > > So, let's use dynamically allocated data, instead. > > > > Signed-off-by: Mauro Carvalho Chehab > > Cc: Guennadi Liakhovetski > > --- > > drivers/media/v4l2-core/v4l2-async.c | 5 - > > 1 file changed, 4 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/media/v4l2-core/v4l2-async.c > > b/drivers/media/v4l2-core/v4l2-async.c > > index c85d69da35bd..071596869036 100644 > > --- a/drivers/media/v4l2-core/v4l2-async.c > > +++ b/drivers/media/v4l2-core/v4l2-async.c > > @@ -189,12 +189,14 @@ void v4l2_async_notifier_unregister(struct > > v4l2_async_notifier *notifier) > > struct v4l2_subdev *sd, *tmp; > > unsigned int notif_n_subdev = notifier->num_subdevs; > > unsigned int n_subdev = min(notif_n_subdev, V4L2_MAX_SUBDEVS); > > - struct device *dev[n_subdev]; > > + struct device **dev; > > int i = 0; > > > > if (!notifier->v4l2_dev) > > return; > > > > + dev = kmalloc(sizeof(*dev) * n_subdev, GFP_KERNEL); > > + > > No check for dev == NULL? Well, what should be done in this case? We could do the changes below: void v4l2_async_notifier_unregister(struct v4l2_async_notifier *notifier) { struct v4l2_subdev *sd, *tmp; unsigned int notif_n_subdev = notifier->num_subdevs; unsigned int n_subdev = min(notif_n_subdev, V4L2_MAX_SUBDEVS); - struct device *dev[n_subdev]; + struct device **dev; int i = 0; if (!notifier->v4l2_dev) return; + dev = kmalloc(sizeof(*dev) * n_subdev, GFP_KERNEL); + if (!dev) { + WARN_ON(true); + return; + } + mutex_lock(&list_lock); list_del(¬ifier->list); list_for_each_entry_safe(sd, tmp, ¬ifier->done, async_list) { dev[i] = get_device(sd->dev); v4l2_async_cleanup(sd); /* If we handled USB devices, we'd have to lock the parent too */ device_release_driver(dev[i++]); if (notifier->unbind) notifier->unbind(notifier, sd, sd->asd); } mutex_unlock(&list_lock); while (i--) { struct device *d = dev[i]; if (d && device_attach(d) < 0) { const char *name = "(none)"; int lock = device_trylock(d); if (lock && d->driver) name = d->driver->name; dev_err(d, "Failed to re-probe to %s\n", name); if (lock) device_unlock(d); } put_device(d); } + kfree(dev); notifier->v4l2_dev = NULL; /* * Don't care about the waiting list, it is initialised and populated * upon notifier registration. */ } EXPORT_SYMBOL(v4l2_async_notifier_unregister); But I suspect that this will cause an OOPS anyway, as the device will be only half-removed. So, it would likely OOPS at device removal or if the device got probed again, at probing time. So, IMHO, we should have at least a WARN_ON() for this case. Do you have a better idea? Regards, Mauro -- 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 v11 02/12] [media] exynos5-fimc-is: Add driver core files
Hi Sakari, Thank you for the review. Will make the required changes. Thanks Arun On Tue, Nov 5, 2013 at 4:51 PM, Sakari Ailus wrote: > Hi Arun, > > Thanks for the patch. A few comments below. > > On Tue, Nov 05, 2013 at 11:42:33AM +0530, Arun Kumar K wrote: >> This driver is for the FIMC-IS IP available in Samsung Exynos5 >> SoC onwards. This patch adds the core files for the new driver. >> >> Signed-off-by: Arun Kumar K >> Signed-off-by: Kilyeon Im >> Reviewed-by: Sylwester Nawrocki >> --- >> drivers/media/platform/exynos5-is/fimc-is-core.c | 410 >> ++ >> drivers/media/platform/exynos5-is/fimc-is-core.h | 132 +++ >> 2 files changed, 542 insertions(+) >> create mode 100644 drivers/media/platform/exynos5-is/fimc-is-core.c >> create mode 100644 drivers/media/platform/exynos5-is/fimc-is-core.h >> >> diff --git a/drivers/media/platform/exynos5-is/fimc-is-core.c >> b/drivers/media/platform/exynos5-is/fimc-is-core.c >> new file mode 100644 >> index 000..2b116d0 >> --- /dev/null >> +++ b/drivers/media/platform/exynos5-is/fimc-is-core.c >> @@ -0,0 +1,410 @@ >> +/* >> + * Samsung EXYNOS5 FIMC-IS (Imaging Subsystem) driver >> +* >> + * Copyright (C) 2013 Samsung Electronics Co., Ltd. >> + * Arun Kumar K >> + * >> + * 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 >> +#include >> +#include >> +#include >> +#include >> +#include >> +#include >> +#include >> +#include >> +#include >> +#include >> +#include >> +#include >> +#include >> +#include >> +#include >> + >> +#include >> +#include >> +#include >> +#include >> +#include >> +#include > > Do you really need all these headers? > >> +#include "fimc-is.h" >> +#include "fimc-is-i2c.h" >> + >> +#define CLK_MCU_ISP_DIV0_FREQ(200 * 100) >> +#define CLK_MCU_ISP_DIV1_FREQ(100 * 100) >> +#define CLK_ISP_DIV0_FREQ(134 * 100) >> +#define CLK_ISP_DIV1_FREQ(68 * 100) >> +#define CLK_ISP_DIVMPWM_FREQ (34 * 100) >> + >> +static const char * const fimc_is_clock_name[] = { >> + [IS_CLK_ISP]= "isp", >> + [IS_CLK_MCU_ISP]= "mcu_isp", >> + [IS_CLK_ISP_DIV0] = "isp_div0", >> + [IS_CLK_ISP_DIV1] = "isp_div1", >> + [IS_CLK_ISP_DIVMPWM]= "isp_divmpwm", >> + [IS_CLK_MCU_ISP_DIV0] = "mcu_isp_div0", >> + [IS_CLK_MCU_ISP_DIV1] = "mcu_isp_div1", >> +}; >> + >> +static void fimc_is_put_clocks(struct fimc_is *is) >> +{ >> + int i; >> + >> + for (i = 0; i < IS_CLK_MAX_NUM; i++) { >> + if (IS_ERR(is->clock[i])) >> + continue; >> + clk_unprepare(is->clock[i]); >> + clk_put(is->clock[i]); >> + is->clock[i] = ERR_PTR(-EINVAL); >> + } >> +} >> + >> +static int fimc_is_get_clocks(struct fimc_is *is) >> +{ >> + struct device *dev = &is->pdev->dev; >> + int i, ret; >> + >> + for (i = 0; i < IS_CLK_MAX_NUM; i++) { >> + is->clock[i] = clk_get(dev, fimc_is_clock_name[i]); >> + if (IS_ERR(is->clock[i])) >> + goto err; >> + ret = clk_prepare(is->clock[i]); >> + if (ret < 0) { >> + clk_put(is->clock[i]); >> + is->clock[i] = ERR_PTR(-EINVAL); >> + goto err; >> + } >> + } >> + return 0; >> +err: >> + fimc_is_put_clocks(is); >> + pr_err("Failed to get clock: %s\n", fimc_is_clock_name[i]); > > How about dev_err() instead? > >> + return -ENXIO; >> +} >> + >> +static int fimc_is_configure_clocks(struct fimc_is *is) >> +{ >> + int i, ret; >> + >> + for (i = 0; i < IS_CLK_MAX_NUM; i++) >> + is->clock[i] = ERR_PTR(-EINVAL); >> + >> + ret = fimc_is_get_clocks(is); >> + if (ret) >> + return ret; >> + >> + /* Set rates */ >> + ret = clk_set_rate(is->clock[IS_CLK_MCU_ISP_DIV0], >> + CLK_MCU_ISP_DIV0_FREQ); >> + if (ret) >> + return ret; >> + ret = clk_set_rate(is->clock[IS_CLK_MCU_ISP_DIV1], >> + CLK_MCU_ISP_DIV1_FREQ); >> + if (ret) >> + return ret; >> + ret = clk_set_rate(is->clock[IS_CLK_ISP_DIV0], CLK_ISP_DIV0_FREQ); >> + if (ret) >> + return ret; >> + ret = clk_set_rate(is->clock[IS_CLK_ISP_DIV1], CLK_ISP_DIV1_FREQ); >> + if (ret) >> + return ret; >> + ret = clk_set_rate(is->clock[IS_CLK_ISP_DIVMPWM], >> + CLK_ISP_DIVMPWM_FREQ); >> + return ret; > > You can return the return value from clk_set_rate() directly here. > >> +} >> + >> +static void fimc_is_pipelines_destroy(struct fimc_is *is) >> +{ >> + int i; >> + >
Re: [PATCH v11 02/12] [media] exynos5-fimc-is: Add driver core files
Hi Arun, Thanks for the patch. A few comments below. On Tue, Nov 05, 2013 at 11:42:33AM +0530, Arun Kumar K wrote: > This driver is for the FIMC-IS IP available in Samsung Exynos5 > SoC onwards. This patch adds the core files for the new driver. > > Signed-off-by: Arun Kumar K > Signed-off-by: Kilyeon Im > Reviewed-by: Sylwester Nawrocki > --- > drivers/media/platform/exynos5-is/fimc-is-core.c | 410 > ++ > drivers/media/platform/exynos5-is/fimc-is-core.h | 132 +++ > 2 files changed, 542 insertions(+) > create mode 100644 drivers/media/platform/exynos5-is/fimc-is-core.c > create mode 100644 drivers/media/platform/exynos5-is/fimc-is-core.h > > diff --git a/drivers/media/platform/exynos5-is/fimc-is-core.c > b/drivers/media/platform/exynos5-is/fimc-is-core.c > new file mode 100644 > index 000..2b116d0 > --- /dev/null > +++ b/drivers/media/platform/exynos5-is/fimc-is-core.c > @@ -0,0 +1,410 @@ > +/* > + * Samsung EXYNOS5 FIMC-IS (Imaging Subsystem) driver > +* > + * Copyright (C) 2013 Samsung Electronics Co., Ltd. > + * Arun Kumar K > + * > + * 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 > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > + > +#include > +#include > +#include > +#include > +#include > +#include Do you really need all these headers? > +#include "fimc-is.h" > +#include "fimc-is-i2c.h" > + > +#define CLK_MCU_ISP_DIV0_FREQ(200 * 100) > +#define CLK_MCU_ISP_DIV1_FREQ(100 * 100) > +#define CLK_ISP_DIV0_FREQ(134 * 100) > +#define CLK_ISP_DIV1_FREQ(68 * 100) > +#define CLK_ISP_DIVMPWM_FREQ (34 * 100) > + > +static const char * const fimc_is_clock_name[] = { > + [IS_CLK_ISP]= "isp", > + [IS_CLK_MCU_ISP]= "mcu_isp", > + [IS_CLK_ISP_DIV0] = "isp_div0", > + [IS_CLK_ISP_DIV1] = "isp_div1", > + [IS_CLK_ISP_DIVMPWM]= "isp_divmpwm", > + [IS_CLK_MCU_ISP_DIV0] = "mcu_isp_div0", > + [IS_CLK_MCU_ISP_DIV1] = "mcu_isp_div1", > +}; > + > +static void fimc_is_put_clocks(struct fimc_is *is) > +{ > + int i; > + > + for (i = 0; i < IS_CLK_MAX_NUM; i++) { > + if (IS_ERR(is->clock[i])) > + continue; > + clk_unprepare(is->clock[i]); > + clk_put(is->clock[i]); > + is->clock[i] = ERR_PTR(-EINVAL); > + } > +} > + > +static int fimc_is_get_clocks(struct fimc_is *is) > +{ > + struct device *dev = &is->pdev->dev; > + int i, ret; > + > + for (i = 0; i < IS_CLK_MAX_NUM; i++) { > + is->clock[i] = clk_get(dev, fimc_is_clock_name[i]); > + if (IS_ERR(is->clock[i])) > + goto err; > + ret = clk_prepare(is->clock[i]); > + if (ret < 0) { > + clk_put(is->clock[i]); > + is->clock[i] = ERR_PTR(-EINVAL); > + goto err; > + } > + } > + return 0; > +err: > + fimc_is_put_clocks(is); > + pr_err("Failed to get clock: %s\n", fimc_is_clock_name[i]); How about dev_err() instead? > + return -ENXIO; > +} > + > +static int fimc_is_configure_clocks(struct fimc_is *is) > +{ > + int i, ret; > + > + for (i = 0; i < IS_CLK_MAX_NUM; i++) > + is->clock[i] = ERR_PTR(-EINVAL); > + > + ret = fimc_is_get_clocks(is); > + if (ret) > + return ret; > + > + /* Set rates */ > + ret = clk_set_rate(is->clock[IS_CLK_MCU_ISP_DIV0], > + CLK_MCU_ISP_DIV0_FREQ); > + if (ret) > + return ret; > + ret = clk_set_rate(is->clock[IS_CLK_MCU_ISP_DIV1], > + CLK_MCU_ISP_DIV1_FREQ); > + if (ret) > + return ret; > + ret = clk_set_rate(is->clock[IS_CLK_ISP_DIV0], CLK_ISP_DIV0_FREQ); > + if (ret) > + return ret; > + ret = clk_set_rate(is->clock[IS_CLK_ISP_DIV1], CLK_ISP_DIV1_FREQ); > + if (ret) > + return ret; > + ret = clk_set_rate(is->clock[IS_CLK_ISP_DIVMPWM], > + CLK_ISP_DIVMPWM_FREQ); > + return ret; You can return the return value from clk_set_rate() directly here. > +} > + > +static void fimc_is_pipelines_destroy(struct fimc_is *is) > +{ > + int i; > + > + for (i = 0; i < is->drvdata->num_instances; i++) > + fimc_is_pipeline_destroy(&is->pipeline[i]); > +} > + > +static int fimc_is_parse_sensor_config(struct fimc_is *is, unsigned int > index, > + struct device_node *node) > +{ > + struct fimc_
[PATCH v11 2/2] [media] exynos5-is: Add media device driver for exynos5 SoCs camera subsystem
From: Shaik Ameer Basha This patch adds a top level media device driver for the Exynos5 SoC series camera subsystem. The driver currently supports processing pipelines involving following IP blocks: * MIPI-CSIS MIPI CSI-2 bus front-end to the FIMC-LITE, image sensors with MIPI CSI-2 bus should be linked with FIMC-LITE through this sub-device. * FIMC-LITE Supports capture interface from device (Sensor, MIPI-CSIS) to memory. Supports interconnection (through internal data FIFO links) between devices like MIPI-CSIS and FIMC-IS. * FIMC-IS Camera ISP with multiple image processing and peripheral devices like I2C or SPI bus controllers. The G-Scaler IP is not yet supported. The media device creates two kinds of pipelines for connecting the above mentioned IP blocks. The pipeline0 contains Sensor, MIPI-CSIS and FIMC-LITE devices and allows to capture image data from external sensor to memory. Pipeline1 uses FIMC-IS components for image processing operations on the captured raw image data and provides scaled YUV image data at its output. Pipeline0 ++ +---+ +---+ ++ | Sensor | --> | MIPI-CSIS | --> | FIMC-LITE | --> | Memory | ++ +---+ +---+ ++ Pipeline1 ++ +-+ +---+ +---+ | Memory | --> | ISP | --> |SCC| --> |SCP| ++ +-+ +---+ +---+ Signed-off-by: Shaik Ameer Basha Signed-off-by: Arun Kumar K [s.nawro...@samsung.com: improved the commit description] Signed-off-by: Sylwester Nawrocki --- drivers/media/platform/exynos5-is/exynos5-mdev.c | 1211 ++ drivers/media/platform/exynos5-is/exynos5-mdev.h | 126 +++ 2 files changed, 1337 insertions(+) create mode 100644 drivers/media/platform/exynos5-is/exynos5-mdev.c create mode 100644 drivers/media/platform/exynos5-is/exynos5-mdev.h diff --git a/drivers/media/platform/exynos5-is/exynos5-mdev.c b/drivers/media/platform/exynos5-is/exynos5-mdev.c new file mode 100644 index 000..1621d94 --- /dev/null +++ b/drivers/media/platform/exynos5-is/exynos5-mdev.c @@ -0,0 +1,1211 @@ +/* + * EXYNOS5 SoC series camera host interface media device driver + * + * Copyright (C) 2013 Samsung Electronics Co., Ltd. + * Shaik Ameer Basha + * Arun Kumar K + * + * This driver is based on exynos4-is media device driver written by + * Sylwester Nawrocki . + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published + * by the Free Software Foundation, either version 2 of the License, + * or (at your option) any later version. + */ + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#include "exynos5-mdev.h" +#include "fimc-is.h" + +#define BAYER_CLK_NAME "sclk_bayer" + +/** + * fimc_pipeline_prepare - update pipeline information with subdevice pointers + * @me: media entity terminating the pipeline + * + * Caller holds the graph mutex. + */ +static void fimc_pipeline_prepare(struct fimc_pipeline *p, + struct media_entity *me) +{ + struct v4l2_subdev *sd; + int i; + + for (i = 0; i < IDX_MAX; i++) + p->subdevs[i] = NULL; + + while (1) { + struct media_pad *pad = NULL; + + /* Find remote source pad */ + for (i = 0; i < me->num_pads; i++) { + struct media_pad *spad = &me->pads[i]; + if (!(spad->flags & MEDIA_PAD_FL_SINK)) + continue; + pad = media_entity_remote_pad(spad); + if (pad) + break; + } + + if (pad == NULL || + media_entity_type(pad->entity) != MEDIA_ENT_T_V4L2_SUBDEV) { + break; + } + sd = media_entity_to_v4l2_subdev(pad->entity); + + switch (sd->grp_id) { + case GRP_ID_FIMC_IS_SENSOR: + case GRP_ID_SENSOR: + p->subdevs[IDX_SENSOR] = sd; + break; + case GRP_ID_CSIS: + p->subdevs[IDX_CSIS] = sd; + break; + case GRP_ID_FLITE: + p->subdevs[IDX_FLITE] = sd; + break; + default: + pr_warn("%s: Unknown subdev grp_id: %#x\n", + __func__, sd->grp_id); + } + me = &sd->entity; + if (me->num_pads == 1) + break; + } + + /* +* For using FIMC-IS firmware controlled sensors, ISP subde
[PATCH v11 1/2] [media] exynos5-is: Adds DT binding documentation
From: Shaik Ameer Basha The patch adds the DT binding doc for exynos5 SoC camera subsystem. Signed-off-by: Shaik Ameer Basha Signed-off-by: Arun Kumar K --- .../bindings/media/exynos5250-camera.txt | 126 1 file changed, 126 insertions(+) create mode 100644 Documentation/devicetree/bindings/media/exynos5250-camera.txt diff --git a/Documentation/devicetree/bindings/media/exynos5250-camera.txt b/Documentation/devicetree/bindings/media/exynos5250-camera.txt new file mode 100644 index 000..09420ba --- /dev/null +++ b/Documentation/devicetree/bindings/media/exynos5250-camera.txt @@ -0,0 +1,126 @@ +Samsung EXYNOS5 SoC Camera Subsystem + + +The Exynos5 SoC Camera subsystem comprises of multiple sub-devices +represented by separate device tree nodes. Currently this includes: FIMC-LITE, +MIPI CSIS and FIMC-IS. + +The sub-device nodes are referenced using phandles in the common 'camera' node +which also includes common properties of the whole subsystem not really +specific to any single sub-device, like common camera port pins or the common +camera bus clocks. + +Common 'camera' node + + +Required properties: + +- compatible : must be "samsung,exynos5250-fimc" +- clocks : list of clock specifiers, corresponding to entries in + the clock-names property +- clock-names : must contain "sclk_bayer" entry +- samsung,csis : list of phandles to the mipi-csis device nodes +- samsung,fimc-lite: list of phandles to the fimc-lite device nodes +- samsung,fimc-is : phandle to the fimc-is device node + +The pinctrl bindings defined in ../pinctrl/pinctrl-bindings.txt must be used +to define a required pinctrl state named "default". + +'parallel-ports' node +- + +This node should contain child 'port' nodes specifying active parallel video +input ports. It includes camera A, camera B and RGB bay inputs. +'reg' property in the port nodes specifies the input type: + 1 - parallel camport A + 2 - parallel camport B + 5 - RGB camera bay + +3, 4 are for MIPI CSI-2 bus and are already described in samsung-mipi-csis.txt + +Image sensor nodes +-- + +The sensor device nodes should be added to their control bus controller (e.g. +I2C0) nodes and linked to a port node in the csis or the parallel-ports node, +using the common video interfaces bindings, defined in video-interfaces.txt. + +Example: + + aliases { + fimc-lite0 = &fimc_lite_0 + }; + + /* Parallel bus IF sensor */ + i2c_0: i2c@1386 { + s5k6aa: sensor@3c { + compatible = "samsung,s5k6aafx"; + reg = <0x3c>; + vddio-supply = <...>; + + clock-frequency = <2400>; + clocks = <...>; + clock-names = "mclk"; + + port { + s5k6aa_ep: endpoint { + remote-endpoint = <&fimc0_ep>; + bus-width = <8>; + hsync-active = <0>; + vsync-active = <1>; + pclk-sample = <1>; + }; + }; + }; + }; + + /* MIPI CSI-2 bus IF sensor */ + s5c73m3: sensor@1a { + compatible = "samsung,s5c73m3"; + reg = <0x1a>; + vddio-supply = <...>; + + clock-frequency = <2400>; + clocks = <...>; + clock-names = "mclk"; + + port { + s5c73m3_1: endpoint { + data-lanes = <1 2 3 4>; + remote-endpoint = <&csis0_ep>; + }; + }; + }; + + camera { + compatible = "samsung,exynos5250-fimc"; + #address-cells = <1>; + #size-cells = <1>; + status = "okay"; + + pinctrl-names = "default"; + pinctrl-0 = <&cam_port_a_clk_active>; + + samsung,csis = <&csis_0>, <&csis_1>; + samsung,fimc-lite = <&fimc_lite_0>, <&fimc_lite_1>, <&fimc_lite_2>; + samsung,fimc-is = <&fimc_is>; + + /* parallel camera ports */ + parallel-ports { + /* camera A input */ + port@1 { + reg = <1>; + camport_a_ep: endpoint { + remote-endpoint = <&s5k6aa_ep>; + bus-width = <8>; + hsync-active = <0>; + vsync-active = <1>; +
Re: [GIT PULL FOR 3.13] Exynos5 SoC FIMC-IS imaging subsystem driver
Hi Sylwester, Ok will resend the media-dev patch after splitting. Thanks Arun On Tue, Nov 5, 2013 at 3:48 PM, Sylwester Nawrocki wrote: > On 05/11/13 05:21, Arun Kumar K wrote: >> Hi Sylwester, >> >> Sorry for the delayed response as I was on leave. >> I will address the comments from Mark today itself and post those DT >> binding patches. > > Hi Arun, > > Thanks, could you also split this patch [1] in two so the DT > binding documentation and the driver are separate and resend it ? > > Regards, > Sylwester > > [1] > http://git.linuxtv.org/snawrocki/samsung.git/commit/4e4cf315ebf98f2553f32cbfe21789126c4ec22c -- 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: [GIT PULL FOR 3.13] Exynos5 SoC FIMC-IS imaging subsystem driver
On 05/11/13 05:21, Arun Kumar K wrote: > Hi Sylwester, > > Sorry for the delayed response as I was on leave. > I will address the comments from Mark today itself and post those DT > binding patches. Hi Arun, Thanks, could you also split this patch [1] in two so the DT binding documentation and the driver are separate and resend it ? Regards, Sylwester [1] http://git.linuxtv.org/snawrocki/samsung.git/commit/4e4cf315ebf98f2553f32cbfe21789126c4ec22c -- 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
[RFC] [PATCH] media: marvell-ccic: use devm to release clk
From: Libin Yang Date: Tue, 5 Nov 2013 16:29:07 +0800 Subject: [PATCH] media: marvell-ccic: use devm to release clk This patch uses devm to release the clks instead of releasing manually. And it adds enable/disable mipi_clk when getting its rate. Signed-off-by: Libin Yang --- drivers/media/platform/marvell-ccic/mmp-driver.c | 39 +- 1 file changed, 8 insertions(+), 31 deletions(-) diff --git a/drivers/media/platform/marvell-ccic/mmp-driver.c b/drivers/media/platform/marvell-ccic/mmp-driver.c index 70cb57f..054507f 100644 --- a/drivers/media/platform/marvell-ccic/mmp-driver.c +++ b/drivers/media/platform/marvell-ccic/mmp-driver.c @@ -142,12 +142,6 @@ static int mmpcam_power_up(struct mcam_camera *mcam) struct mmp_camera *cam = mcam_to_cam(mcam); struct mmp_camera_platform_data *pdata; - if (mcam->bus_type == V4L2_MBUS_CSI2) { - cam->mipi_clk = devm_clk_get(mcam->dev, "mipi"); - if ((IS_ERR(cam->mipi_clk) && mcam->dphy[2] == 0)) - return PTR_ERR(cam->mipi_clk); - } - /* * Turn on power and clocks to the controller. */ @@ -186,12 +180,6 @@ static void mmpcam_power_down(struct mcam_camera *mcam) gpio_set_value(pdata->sensor_power_gpio, 0); gpio_set_value(pdata->sensor_reset_gpio, 0); - if (mcam->bus_type == V4L2_MBUS_CSI2 && !IS_ERR(cam->mipi_clk)) { - if (cam->mipi_clk) - devm_clk_put(mcam->dev, cam->mipi_clk); - cam->mipi_clk = NULL; - } - mcam_clk_disable(mcam); } @@ -292,8 +280,9 @@ void mmpcam_calc_dphy(struct mcam_camera *mcam) return; /* get the escape clk, this is hard coded */ + clk_prepare_enable(cam->mipi_clk); tx_clk_esc = (clk_get_rate(cam->mipi_clk) / 100) / 12; - + clk_disable_unprepare(cam->mipi_clk); /* * dphy[2] - CSI2_DPHY6: * bit 0 ~ bit 7: CK Term Enable @@ -325,19 +314,6 @@ static irqreturn_t mmpcam_irq(int irq, void *data) return IRQ_RETVAL(handled); } -static void mcam_deinit_clk(struct mcam_camera *mcam) -{ - unsigned int i; - - for (i = 0; i < NR_MCAM_CLK; i++) { - if (!IS_ERR(mcam->clk[i])) { - if (mcam->clk[i]) - devm_clk_put(mcam->dev, mcam->clk[i]); - } - mcam->clk[i] = NULL; - } -} - static void mcam_init_clk(struct mcam_camera *mcam) { unsigned int i; @@ -371,7 +347,6 @@ static int mmpcam_probe(struct platform_device *pdev) if (cam == NULL) return -ENOMEM; cam->pdev = pdev; - cam->mipi_clk = NULL; INIT_LIST_HEAD(&cam->devlist); mcam = &cam->mcam; @@ -387,6 +362,11 @@ static int mmpcam_probe(struct platform_device *pdev) mcam->mclk_div = pdata->mclk_div; mcam->bus_type = pdata->bus_type; mcam->dphy = pdata->dphy; + if (mcam->bus_type == V4L2_MBUS_CSI2) { + cam->mipi_clk = devm_clk_get(mcam->dev, "mipi"); + if ((IS_ERR(cam->mipi_clk) && mcam->dphy[2] == 0)) + return PTR_ERR(cam->mipi_clk); + } mcam->mipi_enabled = false; mcam->lane = pdata->lane; mcam->chip_id = MCAM_ARMADA610; @@ -444,7 +424,7 @@ static int mmpcam_probe(struct platform_device *pdev) */ ret = mmpcam_power_up(mcam); if (ret) - goto out_deinit_clk; + return ret; ret = mccic_register(mcam); if (ret) goto out_power_down; @@ -469,8 +449,6 @@ out_unregister: mccic_shutdown(mcam); out_power_down: mmpcam_power_down(mcam); -out_deinit_clk: - mcam_deinit_clk(mcam); return ret; } @@ -482,7 +460,6 @@ static int mmpcam_remove(struct mmp_camera *cam) mmpcam_remove_device(cam); mccic_shutdown(mcam); mmpcam_power_down(mcam); - mcam_deinit_clk(mcam); return 0; } -- 1.7.9.5 -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [REVIEW PATCH 8/9] si4713: move supply list to si4713_platform_data
Hi, On 11/04/13 15:07, edubez...@gmail.com wrote: > Hi, > > On Tue, Oct 15, 2013 at 11:24 AM, Dinesh Ram wrote: >> The supply list is needed by the platform driver, but not by the usb driver. >> So this information belongs to the platform data and should not be hardcoded >> in the subdevice driver. >> >> Signed-off-by: Hans Verkuil > > Dinesh, could you please sign this patch too? > >> --- >> arch/arm/mach-omap2/board-rx51-peripherals.c |7 >> drivers/media/radio/si4713/si4713.c | 52 >> +- >> drivers/media/radio/si4713/si4713.h |3 +- >> include/media/si4713.h |2 + >> 4 files changed, 37 insertions(+), 27 deletions(-) >> >> diff --git a/arch/arm/mach-omap2/board-rx51-peripherals.c >> b/arch/arm/mach-omap2/board-rx51-peripherals.c >> index f6fe388..eae73f7 100644 >> --- a/arch/arm/mach-omap2/board-rx51-peripherals.c >> +++ b/arch/arm/mach-omap2/board-rx51-peripherals.c >> @@ -776,7 +776,14 @@ static struct regulator_init_data rx51_vintdig = { >> }, >> }; >> >> +static const char * const si4713_supply_names[SI4713_NUM_SUPPLIES] = { > > This patch produces the following compilation error: > arch/arm/mach-omap2/board-rx51-peripherals.c:779:47: error: > 'SI4713_NUM_SUPPLIES' undeclared here (not in a function) Hmm, I thought I had compile-tested this, apparently not. Does it compile if you just remove SI4713_NUM_SUPPLIES? It's not necessary here. Regards, Hans > arch/arm/mach-omap2/board-rx51-peripherals.c:785:14: error: bit-field > '' width not an integer constant > arch/arm/mach-omap2/board-rx51-peripherals.c:779:27: warning: > 'si4713_supply_names' defined but not used [-Wunused-variable] > make[1]: *** [arch/arm/mach-omap2/board-rx51-peripherals.o] Error 1 > make: *** [arch/arm/mach-omap2] Error 2 > make: *** Waiting for unfinished jobs > > >> + "vio", >> + "vdd", >> +}; >> + >> static struct si4713_platform_data rx51_si4713_i2c_data >> __initdata_or_module = { >> + .supplies = ARRAY_SIZE(si4713_supply_names), >> + .supply_names = si4713_supply_names, >> .gpio_reset = RX51_FMTX_RESET_GPIO, >> }; >> >> diff --git a/drivers/media/radio/si4713/si4713.c >> b/drivers/media/radio/si4713/si4713.c >> index d297a5b..920dfa5 100644 >> --- a/drivers/media/radio/si4713/si4713.c >> +++ b/drivers/media/radio/si4713/si4713.c >> @@ -44,11 +44,6 @@ MODULE_AUTHOR("Eduardo Valentin >> "); >> MODULE_DESCRIPTION("I2C driver for Si4713 FM Radio Transmitter"); >> MODULE_VERSION("0.0.1"); >> >> -static const char *si4713_supply_names[SI4713_NUM_SUPPLIES] = { >> - "vio", >> - "vdd", >> -}; >> - >> #define DEFAULT_RDS_PI 0x00 >> #define DEFAULT_RDS_PTY0x00 >> #define DEFAULT_RDS_DEVIATION 0x00C8 >> @@ -368,11 +363,12 @@ static int si4713_powerup(struct si4713_device *sdev) >> if (sdev->power_state) >> return 0; >> >> - err = regulator_bulk_enable(ARRAY_SIZE(sdev->supplies), >> - sdev->supplies); >> - if (err) { >> - v4l2_err(&sdev->sd, "Failed to enable supplies: %d\n", err); >> - return err; >> + if (sdev->supplies) { >> + err = regulator_bulk_enable(sdev->supplies, >> sdev->supply_data); >> + if (err) { >> + v4l2_err(&sdev->sd, "Failed to enable supplies: >> %d\n", err); >> + return err; >> + } >> } >> if (gpio_is_valid(sdev->gpio_reset)) { >> udelay(50); >> @@ -396,11 +392,12 @@ static int si4713_powerup(struct si4713_device *sdev) >> if (client->irq) >> err = si4713_write_property(sdev, SI4713_GPO_IEN, >> SI4713_STC_INT | SI4713_CTS); >> - } else { >> - if (gpio_is_valid(sdev->gpio_reset)) >> - gpio_set_value(sdev->gpio_reset, 0); >> - err = regulator_bulk_disable(ARRAY_SIZE(sdev->supplies), >> -sdev->supplies); >> + return err; >> + } >> + if (gpio_is_valid(sdev->gpio_reset)) >> + gpio_set_value(sdev->gpio_reset, 0); >> + if (sdev->supplies) { >> + err = regulator_bulk_disable(sdev->supplies, >> sdev->supply_data); >> if (err) >> v4l2_err(&sdev->sd, >> "Failed to disable supplies: %d\n", err); >> @@ -432,11 +429,13 @@ static int si4713_powerdown(struct si4713_device *sdev) >> v4l2_dbg(1, debug, &sdev->sd, "Device in reset mode\n"); >> if (gpio_is_valid(sdev->gpio_reset)) >> gpio_set_value(sdev->gpio_reset, 0); >> - err = regulator_bulk_disable(ARRAY_SIZE(sdev->supplies), >> -