Re: [PATCH] media: i2c: adv7343: fix the DT binding properties
On Tue, Sep 24, 2013 at 3:03 AM, Stephen Warren swar...@wwwdotorg.org wrote: On 09/23/2013 05:50 AM, Laurent Pinchart wrote: On Monday 23 September 2013 08:18:52 Prabhakar Lad wrote: On Fri, Sep 20, 2013 at 3:22 PM, Sylwester Nawrocki wrote: On 09/20/2013 10:11 AM, Prabhakar Lad wrote: OK I will, just send out a fix up patch which fixes the mismatch between names for the rc-cycle, and later send out a patch which removes the platform data usage for next release with proper DT bindings. I think the binding need to be fully corrected now, I just meant to not touch the board file, i.e. leave non-dt support unchanged. Ok I'm OK with making regulator properties as optional, But still it would change the meaning of what DT is, we know that the VDD/VDD_IO .. etc pins are required properties (but still making them as optional) :-( I think there might several devices where this situation may arise so just thinking of a alternative solution. say we have property 'software-regulator' which takes true/false(0/1) If set to true we make the regulators as required property or else we assume it is handled and ignore it ? I don't think this is a good idea. You would have to add a similar platform data flag for non-dt, it doesn't sound right. I can see two options here: 1. Make the regulator properties mandatory and, e.g. define a fixed voltage GPIO regulator in DT with an empty 'gpio' property. Then pass a phandle to that regulator in the adv7343 *-supply properties. For non-dt similarly a fixed voltage regulator(s) and voltage supplies would need to be defined in the board files. 2. Make the properties optional and use (devm_)regulator_get_optional() calls in the driver (a recently added function). I must admit I don't fully understand description of this function, it currently looks pretty much same as (devm_)regulator_get(). Thus the driver would need to be handling regulator supplies only when non ERR_PTR() is returned from regulator_get_optional() and otherwise assume a non critical error. There is already quite a few example occurrences of regulator_get_optional() usage. Thanks for pointing it I'll choose option 2 and post the patch. Isn't regulator_get_optional() intended for devices that can have supplies unconnected in normal use ? I believe so, yes. Agreed The ADV7343 supplies are mandatory from a hardware point of view, so I think we should use regulator_get(). Otherwise the driver won't be able to tell the difference between a regulator that isn't present yet (for instance because the regulator device/driver hasn't been probed yet), which should result in deferred probing, and an always-on regulator that has been left out. So I think you want to make the supply properties mandatory in DT (since some form of supply is mandatory in HW), yet make the driver support broken DTs which don't have those properties, by error-checking the return value from regulator_get(). You might want to put a note into DT saying that a previous version of the binding didn't require those supply properties, so they may be missing from older DTs. OK, I'll fix it and repost the patch. Regards, --Prabhakar -- 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 v4] media: st-rc: Add ST remote control driver
Thanks Stephen, On 23/09/13 21:40, Stephen Warren wrote: On 09/19/2013 02:59 AM, Srinivas KANDAGATLA wrote: This patch adds support to ST RC driver, which is basically a IR/UHF receiver and transmitter. This IP (IRB) is common across all the ST parts for settop box platforms. IRB is embedded in ST COMMS IP block. It supports both Rx Tx functionality. In this driver adds only Rx functionality via LIRC codec. diff --git a/Documentation/devicetree/bindings/media/st-rc.txt b/Documentation/devicetree/bindings/media/st-rc.txt +Required properties: +- compatible: should contain st,comms-irb. +- reg: base physical address of the controller and length of memory +mapped region. Nits: The indentation is a little odd here; I'd expect the - to be in column 1, at least that's how most other binding docs are written. Not a big deal though. It'd be nice to indent the continuation lines (e.g. mapped region) a bit more so it's easier to see where each new entry starts. There are two spaces in mapped region. I should have noticed it... I will fix it in next version. +- rx-mode: can be infrared or uhf. rx-mode should be present iff the + rx pins are wired up. +- tx-mode: should be infrared. tx-mode should be present iff the tx + pins are wired up. Should those property names be prefixed with st,; I assume they're specific to this binding rather than something generic that applies to all IR controller bindings? If you expect them to be generic, it's fine. Officially these bindings are not specified in ePAPR specs but I see no reason for not having these properties as generic ones. Are you ok with that? +- clocks : phandle with clock-specifier pair. For what clock? Its Clock for IRB block. I will add this in next version. Thanks, srini -- 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: V2: Agenda for the Edinburgh mini-summit
On Mon, Sep 23, 2013 at 10:27:06PM +0200, Sylwester Nawrocki wrote: On 09/23/2013 06:37 PM, Oliver Schinagl wrote: On 09/23/13 16:45, Sylwester Nawrocki wrote: Hi, I would like to have a short discussion on LED flash devices support in the kernel. Currently there are two APIs: the V4L2 and LED class API exposed by the kernel, which I believe is not good from user space POV. Generic applications will need to implement both APIs. I think we should decide whether to extend the led class API to add support for more advanced LED controllers there or continue to use the both APIs with overlapping functionality. There has been some discussion about this on the ML, but without any consensus reached [1]. What about the linux-pwm framework and its support for the backlight via dts? Or am I talking way to uninformed here. Copying backlight to flashlight with some minor modification sounds sensible in a way... I'd assume we don't need yet another user interface for the LEDs ;) AFAICS the PWM subsystem exposes pretty much raw interface in sysfs. The PWM LED controllers are already handled in the leds-class API, there is the leds_pwm driver (drivers/leds/leds-pwm.c). I'm adding linux-pwm and linux-leds maintainers at Cc so someone may correct me if I got anything wrong. The PWM subsystem is most definitely not a good fit for this. The only thing it provides is a way for other drivers to access a PWM device and use it for some specific purpose (pwm-backlight, leds-pwm). The sysfs support is a convenience for people that needs to use a PWM in a way for which no driver framework exists, or for which it doesn't make sense to write a driver. Or for testing. Presumably, what we need is a few enhancements to support in a standard way devices like MAX77693, LM3560 or MAX8997. There is already a led class driver for the MAX8997 LED controller (drivers/leds/leds-max8997.c), but it uses some device-specific sysfs attributes. Thus similar devices are currently being handled by different subsystems. The split between the V4L2 Flash and the leds class API WRT to Flash LED controller drivers is included in RFC [1], it seems still up to date. [1] http://www.spinics.net/lists/linux-leds/msg00899.html Perhaps it would make sense for V4L2 to be able to use a LED as exposed by the LED subsystem and wrap it so that it can be integrated with V4L2? If functionality is missing from the LED subsystem I suppose that could be added. If I understand correctly, the V4L2 subsystem uses LEDs as flashes for camera devices. I can easily imagine that there are devices out there which provide functionality beyond what a regular LED will provide. So perhaps for things such as mobile phones, which typically use a plain LED to illuminate the surroundings, an LED wrapped into something that emulates the flash functionality could work. But I doubt that the LED subsystem is a good fit for anything beyond that. Thierry pgp5r15Q9vljn.pgp Description: PGP signature
Re: V2: Agenda for the Edinburgh mini-summit
Resending using Bryan's correct email address. On Mon, Sep 23, 2013 at 10:27:06PM +0200, Sylwester Nawrocki wrote: On 09/23/2013 06:37 PM, Oliver Schinagl wrote: On 09/23/13 16:45, Sylwester Nawrocki wrote: Hi, I would like to have a short discussion on LED flash devices support in the kernel. Currently there are two APIs: the V4L2 and LED class API exposed by the kernel, which I believe is not good from user space POV. Generic applications will need to implement both APIs. I think we should decide whether to extend the led class API to add support for more advanced LED controllers there or continue to use the both APIs with overlapping functionality. There has been some discussion about this on the ML, but without any consensus reached [1]. What about the linux-pwm framework and its support for the backlight via dts? Or am I talking way to uninformed here. Copying backlight to flashlight with some minor modification sounds sensible in a way... I'd assume we don't need yet another user interface for the LEDs ;) AFAICS the PWM subsystem exposes pretty much raw interface in sysfs. The PWM LED controllers are already handled in the leds-class API, there is the leds_pwm driver (drivers/leds/leds-pwm.c). I'm adding linux-pwm and linux-leds maintainers at Cc so someone may correct me if I got anything wrong. The PWM subsystem is most definitely not a good fit for this. The only thing it provides is a way for other drivers to access a PWM device and use it for some specific purpose (pwm-backlight, leds-pwm). The sysfs support is a convenience for people that needs to use a PWM in a way for which no driver framework exists, or for which it doesn't make sense to write a driver. Or for testing. Presumably, what we need is a few enhancements to support in a standard way devices like MAX77693, LM3560 or MAX8997. There is already a led class driver for the MAX8997 LED controller (drivers/leds/leds-max8997.c), but it uses some device-specific sysfs attributes. Thus similar devices are currently being handled by different subsystems. The split between the V4L2 Flash and the leds class API WRT to Flash LED controller drivers is included in RFC [1], it seems still up to date. [1] http://www.spinics.net/lists/linux-leds/msg00899.html Perhaps it would make sense for V4L2 to be able to use a LED as exposed by the LED subsystem and wrap it so that it can be integrated with V4L2? If functionality is missing from the LED subsystem I suppose that could be added. If I understand correctly, the V4L2 subsystem uses LEDs as flashes for camera devices. I can easily imagine that there are devices out there which provide functionality beyond what a regular LED will provide. So perhaps for things such as mobile phones, which typically use a plain LED to illuminate the surroundings, an LED wrapped into something that emulates the flash functionality could work. But I doubt that the LED subsystem is a good fit for anything beyond that. Thierry pgpLiep3md6DX.pgp Description: PGP signature
Re: [PATCH v5 1/4] media: s5p-tv: Replace mxr_ macro by default dev_
Hi, On 09/23/2013 05:48 PM, Bartlomiej Zolnierkiewicz wrote: Hi Tomasz, On Monday, September 23, 2013 04:50:01 PM Tomasz Stanislawski wrote: Hello, May I ask what is the rationale for this patch? To reduce a few lines of code? This patch makes source code more generic-like and easier to follow (mxd_r* more generic(-like?) - NOT. Using mxr_ macros is a more generic way to produce logs because one can change only one line to change error format for the whole module. For example, in case of mxr_ family, a patch adding function name to debug message would require just a few lines patch. Using in case of dev_ family, one has to produce 200-lines of highly conflicting patch. 'easier to follow' - MAYBE. I agree that some people may prefer to see more directly what is happening, but tell me if you really consider line: mxr_dbg(mdev, this is debug\n); as a very confusing and obfuscated piece of code. COME ON! Regards, TS macros currently only obfuscate the code and make them harder to read for everybody, maybe besides the original driver author ;). Removal of few superfluous lines of code is just a bonus. Or to give up possibility of changing message format in just one place? For over two and half year (since s5p-tv driver introduction in Feb 2011) such change was not needed and it doesn't seem that keeping the code to allow such possibility is worth an added code obfuscation. Besides you can easily script a change to message format so in practice I don't see a real advantage of keeping non-standard messaging macros just for easing a potential message format conversion. I could see migrating from mxr_* to pr_* could seen as the fix, but not this. Such migration seems to be pointless as you would have to add an extra argument to pr_* to not lose the device information. There is something called pr_fmt. It may help with mentioned issue. Best regards, -- Bartlomiej Zolnierkiewicz Samsung RD Institute Poland Samsung Electronics Waiting for reply, Tomasz Stanislawski On 09/21/2013 05:00 PM, Mateusz Krawczuk wrote: Replace mxr_dbg, mxr_info and mxr_warn by generic solution. Signed-off-by: Mateusz Krawczuk m.krawc...@partner.samsung.com Signed-off-by: Kyungmin Park kyungmin.p...@samsung.com --- drivers/media/platform/s5p-tv/mixer.h | 12 --- drivers/media/platform/s5p-tv/mixer_drv.c | 47 ++- drivers/media/platform/s5p-tv/mixer_grp_layer.c | 2 +- drivers/media/platform/s5p-tv/mixer_reg.c | 6 +- drivers/media/platform/s5p-tv/mixer_video.c | 100 drivers/media/platform/s5p-tv/mixer_vp_layer.c | 2 +- 6 files changed, 78 insertions(+), 91 deletions(-) -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] media: i2c: adv7343: fix the DT binding properties
On Monday 23 September 2013 15:33:10 Stephen Warren wrote: On 09/23/2013 05:50 AM, Laurent Pinchart wrote: On Monday 23 September 2013 08:18:52 Prabhakar Lad wrote: On Fri, Sep 20, 2013 at 3:22 PM, Sylwester Nawrocki wrote: On 09/20/2013 10:11 AM, Prabhakar Lad wrote: OK I will, just send out a fix up patch which fixes the mismatch between names for the rc-cycle, and later send out a patch which removes the platform data usage for next release with proper DT bindings. I think the binding need to be fully corrected now, I just meant to not touch the board file, i.e. leave non-dt support unchanged. Ok I'm OK with making regulator properties as optional, But still it would change the meaning of what DT is, we know that the VDD/VDD_IO .. etc pins are required properties (but still making them as optional) :-( I think there might several devices where this situation may arise so just thinking of a alternative solution. say we have property 'software-regulator' which takes true/false(0/1) If set to true we make the regulators as required property or else we assume it is handled and ignore it ? I don't think this is a good idea. You would have to add a similar platform data flag for non-dt, it doesn't sound right. I can see two options here: 1. Make the regulator properties mandatory and, e.g. define a fixed voltage GPIO regulator in DT with an empty 'gpio' property. Then pass a phandle to that regulator in the adv7343 *-supply properties. For non-dt similarly a fixed voltage regulator(s) and voltage supplies would need to be defined in the board files. 2. Make the properties optional and use (devm_)regulator_get_optional() calls in the driver (a recently added function). I must admit I don't fully understand description of this function, it currently looks pretty much same as (devm_)regulator_get(). Thus the driver would need to be handling regulator supplies only when non ERR_PTR() is returned from regulator_get_optional() and otherwise assume a non critical error. There is already quite a few example occurrences of regulator_get_optional() usage. Thanks for pointing it I'll choose option 2 and post the patch. Isn't regulator_get_optional() intended for devices that can have supplies unconnected in normal use ? I believe so, yes. The ADV7343 supplies are mandatory from a hardware point of view, so I think we should use regulator_get(). Otherwise the driver won't be able to tell the difference between a regulator that isn't present yet (for instance because the regulator device/driver hasn't been probed yet), which should result in deferred probing, and an always-on regulator that has been left out. So I think you want to make the supply properties mandatory in DT (since some form of supply is mandatory in HW), yet make the driver support broken DTs which don't have those properties, by error-checking the return value from regulator_get(). You might want to put a note into DT saying that a previous version of the binding didn't require those supply properties, so they may be missing from older DTs. Are there such devices in the wild ? -- 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 v5 1/4] media: s5p-tv: Replace mxr_ macro by default dev_
Hi Tomasz, On Tuesday, September 24, 2013 11:43:53 AM Tomasz Stanislawski wrote: Hi, On 09/23/2013 05:48 PM, Bartlomiej Zolnierkiewicz wrote: Hi Tomasz, On Monday, September 23, 2013 04:50:01 PM Tomasz Stanislawski wrote: Hello, May I ask what is the rationale for this patch? To reduce a few lines of code? This patch makes source code more generic-like and easier to follow (mxd_r* more generic(-like?) - NOT. Using mxr_ macros is a more generic way to produce logs Using mxr_* macros is not more generic, don't be silly. :) because one can change only one line to change error format for the whole module. For example, in case of mxr_ family, a patch adding function name to debug message would require just a few lines patch. Using in case of dev_ family, one has to produce 200-lines of highly conflicting patch. * For over two and half year since the s5p-tv driver introduction there was no such need and it is very _unlikely_ that it will be one in the future. * Optimizing for the completely _theorethical_ future patches size is just over-enginneering IMHO. 'easier to follow' - MAYBE. I agree that some people may prefer to see more directly what is happening, but tell me if you really consider line: mxr_dbg(mdev, this is debug\n); as a very confusing and obfuscated piece of code. COME ON! It is confusing becuase you have to lookup what mxr_dbg() actually does, extra time needs to be spent on seeing that the mxr_* macros are just a needless wrappers. You also have to recall it from the memory or look it up again if you come back to the code some time later. Just stop arguing about this trivial change and Ack Mateusz's patch already. :) PS Please try to not top post, thanks. Best regards, -- Bartlomiej Zolnierkiewicz Samsung RD Institute Poland Samsung Electronics Regards, TS macros currently only obfuscate the code and make them harder to read for everybody, maybe besides the original driver author ;). Removal of few superfluous lines of code is just a bonus. Or to give up possibility of changing message format in just one place? For over two and half year (since s5p-tv driver introduction in Feb 2011) such change was not needed and it doesn't seem that keeping the code to allow such possibility is worth an added code obfuscation. Besides you can easily script a change to message format so in practice I don't see a real advantage of keeping non-standard messaging macros just for easing a potential message format conversion. I could see migrating from mxr_* to pr_* could seen as the fix, but not this. Such migration seems to be pointless as you would have to add an extra argument to pr_* to not lose the device information. There is something called pr_fmt. It may help with mentioned issue. Best regards, -- Bartlomiej Zolnierkiewicz Samsung RD Institute Poland Samsung Electronics Waiting for reply, Tomasz Stanislawski On 09/21/2013 05:00 PM, Mateusz Krawczuk wrote: Replace mxr_dbg, mxr_info and mxr_warn by generic solution. Signed-off-by: Mateusz Krawczuk m.krawc...@partner.samsung.com Signed-off-by: Kyungmin Park kyungmin.p...@samsung.com --- drivers/media/platform/s5p-tv/mixer.h | 12 --- drivers/media/platform/s5p-tv/mixer_drv.c | 47 ++- drivers/media/platform/s5p-tv/mixer_grp_layer.c | 2 +- drivers/media/platform/s5p-tv/mixer_reg.c | 6 +- drivers/media/platform/s5p-tv/mixer_video.c | 100 drivers/media/platform/s5p-tv/mixer_vp_layer.c | 2 +- 6 files changed, 78 insertions(+), 91 deletions(-) -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [media-workshop] V2: Agenda for the Edinburgh mini-summit
Hi Oliver, On Monday 23 September 2013 18:37:48 Oliver Schinagl wrote: On 09/23/13 16:45, Sylwester Nawrocki wrote: Hi, I would like to have a short discussion on LED flash devices support in the kernel. Currently there are two APIs: the V4L2 and LED class API exposed by the kernel, which I believe is not good from user space POV. Generic applications will need to implement both APIs. I think we should decide whether to extend the led class API to add support for more advanced LED controllers there or continue to use the both APIs with overlapping functionality. There has been some discussion about this on the ML, but without any consensus reached [1]. What about the linux-pwm framework and its support for the backlight via dts? Or am I talking way to uninformed here. Copying backlight to flashlight with some minor modification sounds sensible in a way... Flash has more advanced requirements, such as internal/external trigger support, hardware timings control, overheat prevention, ... Furthermore, a flash API should not be limited to LEDs, as Xeon flashes are common and similar enough to be supported by the same API. I would like to see the two APIs merged in a way, at least for flash devices. -- 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: [media-workshop] V2: Agenda for the Edinburgh mini-summit
Hi Hans, On Tuesday 10 September 2013 11:34:32 Hans Verkuil wrote: I have collected all the ideas up to now in a V2 of the agenda. The items are grouped by the person(s) that suggested them. As done in the past those who suggested a topic and who will attend the mini-summit are expected to prepare for it, perhaps making a (very) small presentation if necessary. [snip] As it stands I don't think it will be possible to handle it all in one day. In particular the codec problem as mentioned by Oliver et al needs a lot of time. Should we set aside a separate day for just this? October 21 or 22 would work for me. I would really like to get some feedback on this. If we decide to go for a second day for this topic, then I can see if I can get a room. It looks like there is a lot of interest in getting this sorted, so brainstorming for a day might be quite useful. I believe a second day would indeed be very useful. As the ARM kernel summit will take place on the 22nd and 23rd, I would vote for brainstorming on the 21st. Note: my email availability will be limited in the next two weeks, especially next week, as I am abroad (LinuxCon and LPC). -- 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: [media-workshop] V2: Agenda for the Edinburgh mini-summit
On Tue 24 September 2013 12:59:44 Laurent Pinchart wrote: Hi Hans, On Tuesday 10 September 2013 11:34:32 Hans Verkuil wrote: I have collected all the ideas up to now in a V2 of the agenda. The items are grouped by the person(s) that suggested them. As done in the past those who suggested a topic and who will attend the mini-summit are expected to prepare for it, perhaps making a (very) small presentation if necessary. [snip] As it stands I don't think it will be possible to handle it all in one day. In particular the codec problem as mentioned by Oliver et al needs a lot of time. Should we set aside a separate day for just this? October 21 or 22 would work for me. I would really like to get some feedback on this. If we decide to go for a second day for this topic, then I can see if I can get a room. It looks like there is a lot of interest in getting this sorted, so brainstorming for a day might be quite useful. I believe a second day would indeed be very useful. As the ARM kernel summit will take place on the 22nd and 23rd, I would vote for brainstorming on the 21st. I'm working on the agenda and I think we have enough time to go through all the items in one day. It would be great if you can be present for either the morning or afternoon for the mini-summit. I have about 4 hours worth of topics for which your input would be much appreciated. I would be happy to add another brainstorming day, but since Hugues won't be available on the 21st we can only do limited codec discussions, so we would need other topics as well. Proposals are welcome. Regards, Hans -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Revert V4L/DVB: uvc: Enable USB autosuspend by default on uvcvideo
Hi Adam, On Thursday 13 June 2013 17:56:47 Adam Lee wrote: On Thu, Apr 25, 2013 at 02:33:06PM +0800, Adam Lee wrote: On Wed, Apr 24, 2013 at 11:17:52AM +0200, Laurent Pinchart wrote: On Wednesday 24 April 2013 15:57:19 adam@canonical.com wrote: From: Adam Lee adam@canonical.com This reverts commit 3dae8b41dc5651f8eb22cf310e8b116480ba25b7. 1, I do have a Chicony webcam, implements autosuspend in a broken way, make `poweroff` performs rebooting when its autosuspend enabled. 2, There are other webcams which don't support autosuspend too, like https://patchwork.kernel.org/patch/2356141/ 3, kernel removed USB_QUIRK_NO_AUTOSUSPEND in a691efa9888e71232dfb4088fb8a8304ffc7b0f9, because autosuspend is disabled by default. So, we need to disable autosuspend in uvcvideo, maintaining a quirk list only for uvcvideo is not a good idea. I've received very few bug reports about broken auto-suspend support in UVC devices. Most of them could be solved by setting the RESET_RESUME quirk in USB core, only the Creative Live! Cam Optia AF required a quirk in the uvcvideo driver. I would thus rather use the available quirks (USB_QUIRK_RESET_RESUME if possible, UVC_QUIRK_DISABLE_AUTOSUSPEND otherwise) than killing power management for the vast majority of webcams that behave correctly. Here comes another one, integrated Chicony webcam 04f2:b39f, its autosuspend makes `poweroff` performs rebooting at the laptop I'm working on. That's a pretty bad one :-/ Do you have any idea why it occurs ? I tried USB_QUIRK_RESET_RESUME, not helping. The quirks list will go longer and longer absolutely, do uvcvideo wanna maintain that? And why only uvcvideo do this in kernel space which against general USB module? I still suggest we disable it by default, people can enable it in udev just like almost all distroes do for udisks. Please consider about it. Any comment of this? I still think it's a risk to enable autosuspend in uvcvideo by default, there must be users meeting weird issues but didn't report to you becaue they didn't figured out the cause. I've discussed this issue during LPC last week, and I still believe we should enable auto-suspend. The feature really saves power, without it my C910 Logitech webcam gets hot even when unused. If we disable auto-suspend by default and enable it from userspace only a handful of devices will get auto-suspended. Unless we can get distros to automatically test auto-suspend on unknown webcam models and report the results to update a central data base (which would grow much bigger than a quirks list in the driver in my opinion), disabling auto-suspend would be a serious regression. Any comment from other developers from the mailing lists ? -- 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] driver: firewire: remove unnecessary #if 0 code
On Sep 22 Govindarajulu Varadarajan wrote: Signed-off-by: Govindarajulu Varadarajan govindarajul...@gmail.com --- drivers/media/firewire/firedtv-avc.c | 41 1 file changed, 41 deletions(-) diff --git a/drivers/media/firewire/firedtv-avc.c b/drivers/media/firewire/firedtv-avc.c index d1a1a13..786e273 100644 --- a/drivers/media/firewire/firedtv-avc.c +++ b/drivers/media/firewire/firedtv-avc.c @@ -912,37 +912,6 @@ void avc_remote_ctrl_work(struct work_struct *work) avc_register_remote_control(fdtv); } -#if 0 /* FIXME: unused */ -int avc_tuner_host2ca(struct firedtv *fdtv) -{ - struct avc_command_frame *c = (void *)fdtv-avc_data; - int ret; - - mutex_lock(fdtv-avc_mutex); - - c-ctype = AVC_CTYPE_CONTROL; - c-subunit = AVC_SUBUNIT_TYPE_TUNER | fdtv-subunit; - c-opcode = AVC_OPCODE_VENDOR; - - c-operand[0] = SFE_VENDOR_DE_COMPANYID_0; - c-operand[1] = SFE_VENDOR_DE_COMPANYID_1; - c-operand[2] = SFE_VENDOR_DE_COMPANYID_2; - c-operand[3] = SFE_VENDOR_OPCODE_HOST2CA; - c-operand[4] = 0; /* slot */ - c-operand[5] = SFE_VENDOR_TAG_CA_APPLICATION_INFO; /* ca tag */ - clear_operands(c, 6, 8); - - fdtv-avc_data_length = 12; - ret = avc_write(fdtv); - - /* FIXME: check response code? */ - - mutex_unlock(fdtv-avc_mutex); - - return ret; -} -#endif - static int get_ca_object_pos(struct avc_response_frame *r) { int length = 1; @@ -955,16 +924,6 @@ static int get_ca_object_pos(struct avc_response_frame *r) static int get_ca_object_length(struct avc_response_frame *r) { -#if 0 /* FIXME: unused */ - int size = 0; - int i; - - if (r-operand[7] 0x80) - for (i = 0; i (r-operand[7] 0x7f); i++) { - size = 8; - size += r-operand[8 + i]; - } -#endif return r-operand[7]; } Hi Henrik, get_ca_object_pos() is used in avc_ca_get_mmi() when the AVC response frame is inspected. Do you know whether we should fix up this code for support of length 127, or can we just delete this #if 0 block? -- Stefan Richter -=-===-= =--= ==--- http://arcgraph.de/sr/ -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5 1/4] media: s5p-tv: Replace mxr_ macro by default dev_
On 09/23/2013 07:44 PM, Joe Perches wrote: On Mon, 2013-09-23 at 17:48 +0200, Bartlomiej Zolnierkiewicz wrote: On Monday, September 23, 2013 04:50:01 PM Tomasz Stanislawski wrote: May I ask what is the rationale for this patch? To reduce a few lines of code? This patch makes source code more generic-like and easier to follow (mxd_r* macros currently only obfuscate the code and make them harder to read for everybody, maybe besides the original driver author ;). Removal of few superfluous lines of code is just a bonus. I don't see any significant issue with this change. Using generic mechanisms is good. Hi Joe, Sorry for flaming but please let me explain reasons of my opposition to this patch. 1. It is true that there was no change in mixer messages for 2.5 year in MAINLINE. But sometimes I used modification of mxr_ macros while testing the driver. Therefore those macros are useful for me. 2. The other problem with this patch is its high 'conflictness' during merging. Unfortunately, sometimes I have to use s5p-tv on platform and configuration that is only supported in older versions of the kernel + some integration patches. The s5p-tv differs from mainline version in those kernels. Therefore I would need to keep two versions of patches, one for old and another one for new kernel. Or backport the 'cleanup patch' and all experimental patches above it. 3. As I understand the coding guidelines asked to use dev_* to ensure that all error messages have information about the device. There is no change in format of errors after this patch. So they do not change anything from userland point of view. 4. I looked for other files where macro for dev_err is used. I tried following shell command on v3.12-rc2. git grep -A1 _err( | grep -A1 '#define' | grep -B1 dev_err then processing results using grep -v ^-- | cut -d: -f 1 | sort -u | wc produced 55 files. Among them, the files below makes use of a macro that is directly expanded to dev_err(dev_ptr, fmt, ...) without any changes in format. drivers/firewire/ohci.c drivers/gpu/drm/i2c/ch7006_priv.h drivers/gpu/drm/i2c/sil164_drv.c drivers/infiniband/hw/mthca/mthca_dev.h drivers/infiniband/hw/qib/qib.h drivers/media/platform/marvell-ccic/cafe-driver.c drivers/media/platform/marvell-ccic/mcam-core.c drivers/media/platform/s5p-tv/mixer.h drivers/media/platform/via-camera.c drivers/mtd/devices/docg3.h drivers/net/ethernet/broadcom/bgmac.h drivers/net/ethernet/chelsio/cxgb3/common.h drivers/net/ethernet/intel/e1000/e1000.h drivers/net/ethernet/intel/ixgbe/ixgbe_common.h drivers/net/ethernet/mellanox/mlx4/mlx4.h drivers/net/wireless/iwlegacy/common.h drivers/pci/hotplug/pciehp.h drivers/pci/hotplug/shpchp.h drivers/remoteproc/ste_modem_rproc.c drivers/scsi/csiostor/csio_hw.h drivers/staging/fwserial/fwserial.c drivers/usb/atm/usbatm.h drivers/usb/host/ehci.h drivers/usb/host/fhci.h drivers/usb/host/fotg210-hcd.c drivers/usb/host/fusbh200-hcd.c drivers/usb/host/ohci.h drivers/usb/host/oxu210hp-hcd.c drivers/usb/host/xhci.h include/linux/hid.h include/net/cfg80211.h Other files makes only cosmetic changes to format, so they might still be worth to be 'demacronized'. So I think we can consider that macros wrapping dev_* is still a widely used technique so I ask for a good reason before changing the driver. If one still would like to continue a 'dev_* cleanup crusade' then I kindly ask to create a big patchset that fixes all over mentioned files. If most of their maintainers accepts the patches I promise to accept it in s5p-tv. Currently, due to mentioned reason the patch is not a cleanup-up for me. And since I am still a maintainer of this god-forgotten driver I am going NACK this patch because it makes my work more difficult and because this patch provides only (if any) relative aesthetic gain. However this patch can be saved a little. (see below) Few trivial nits: I'd remove the trailing periods from some of the messages at the same time. Function tracing is better done by the function tracing mechanism built in to the kernel. Removing the dev_dbg(dev, %s: enter\n, __func__) lines would be good too. Maybe look at the message levels of more of these logging messages and determine which are actually useful and what is mostly noise and should be dev_dbg or deleted altogether. I agree that most of debugs in form mxr_dbg(layer-mdev, %s:%d\n, __func__, __LINE__); are obfuscation. So removal of such lines is a cleanup and provides some gain and I can ACK this kind of change. Moreover, I agree that some mxr_info() should be changed mxr_dbg(). I ask Mateusz to modify the 'cleanup' patch to remove only useless mxr_dbg() and mxr_info() but to keep mxr_* macros intact. Regards, Tomasz Stanislawski diff --git a/drivers/media/platform/s5p-tv/mixer_drv.c b/drivers/media/platform/s5p-tv/mixer_drv.c @@ -59,7 +59,7 @@ void mxr_streamer_get(struct mxr_device *mdev) { mutex_lock(mdev-mutex); ++mdev-n_streamer; -
Re: [PATCH] V4L: Drop meaningless video_is_registered() call in v4l2_open()
On Thu 12 September 2013 12:19:29 Sylwester Nawrocki wrote: Hi Hans, On 09/11/2013 04:01 PM, Hans Verkuil wrote: On 09/11/2013 03:07 PM, Sylwester Nawrocki wrote: On 09/09/2013 11:07 AM, Hans Verkuil wrote: On 09/06/2013 12:33 AM, Sylwester Nawrocki wrote: On 08/07/2013 07:49 PM, Hans Verkuil wrote: On 08/07/2013 06:49 PM, Sylwester Nawrocki wrote: On 08/02/2013 03:00 PM, Hans Verkuil wrote: On 08/02/2013 02:27 PM, Sylwester Nawrocki wrote: [...] So there is lots of things that may fail after first video node is registered, and on which open() may be called immediately. The only solution I know off-hand to handle this safely is to unregister all nodes if some fail, but to return 0 in the probe function. If an open() succeeded, then that will just work until the node is closed, at which point the v4l2_device release() is called and you can cleanup. Another solution would be to properly implement struct video_device::release() callback and in video device open() do video_is_registered() check while holding the driver private video device lock. Also video_unregister_device() would need to be called while holding the video device lock. How would that help? By not allowing video_unregister_device() call in the middle of the file op and serializing whatever does video device unregistration with the file ops. I'm not sure it is possible in all cases. That does not actually help you. An open() call may succeed during probe(), but if an error occurs in the probe() afterwards and the device is unregistered and all internal data released, you still crash. While probe() is in progress you want to block all open()s. Since all video_device structs now have a v4l2_device pointer, what about doing this in v4l2_open(): if (test_bit(V4L2_DEV_FL_IN_PROBE, vdev-v4l2_dev-flags)) return -EAGAIN; It's not ideal, but this allows drivers with a complex probe sequence to set the flag and clear it at the end. While it is set, all opens will return EAGAIN. Regards, Hans For ioctls it's already party taken care of by video_is_registered() check while holding the video dev mutex. For other file ops drivers currently need to do various checks to ensure that all driver private data/resources that may be needed in the file operations callbacks are in place. [...] Is this 'could fail', or 'I have seen it fail'? I have never seen problems in probe() with node creation. The only reason I know of why creating a node might fail is being out-of-memory. In my case it was top level driver that was triggering device node creation in its probe() and if, e.g. some of sub-device's driver was missing, it called, through subdev internal unregistered(), op video_unregister_device(), but also media_entity_cleanup() which seemed to be the source of trouble. Device nodes should always be created at the very end after all other setup actions (like loaded subdev drivers) have been done successfully. From your description it seems that device nodes were created earlier? Yes, let me explain in what configuration it happens (perhaps looking at fimc_md_probe() function at drives/media/platform/exynos4-is/media-dev.c makes it easier to understand; the version closer to what we work with currently is available at [1]). There are multiple platform devices that in their driver's probe() just initialize the driver private data and platform device resources, like irq, IO register, etc. There is also a top level driver (struct v4l2_device) that walks through Device Tree and registers those platform devices as v4l2 subdevs. While doing that video nodes are created in some subdevs' .registered() callbacks. After that image sensors are being registered. Some platform devices need to be registered before image sensors, due to resource dependencies. E.g. some device need to be activated so clock for an image sensor is available at an SoC output pin. So this is a bit more complex issue than with a single device, a single driver and its probe() interaction with the file ops. [1] git.linuxtv.org/snawrocki/samsung.git/v3.11-rc2-dts-exynos4-is-clk -- 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 -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5 1/4] media: s5p-tv: Replace mxr_ macro by default dev_
Em Tue, 24 Sep 2013 12:33:34 +0200 Bartlomiej Zolnierkiewicz b.zolnier...@samsung.com escreveu: Hi Tomasz, On Tuesday, September 24, 2013 11:43:53 AM Tomasz Stanislawski wrote: Hi, On 09/23/2013 05:48 PM, Bartlomiej Zolnierkiewicz wrote: Hi Tomasz, On Monday, September 23, 2013 04:50:01 PM Tomasz Stanislawski wrote: Hello, May I ask what is the rationale for this patch? To reduce a few lines of code? This patch makes source code more generic-like and easier to follow (mxd_r* more generic(-like?) - NOT. Using mxr_ macros is a more generic way to produce logs Using mxr_* macros is not more generic, don't be silly. :) Let me give my 2 cents on it: it used to be common on media drivers to define their own print macros. The rationale for that is because the existing macros, on that time (kernel 2.2), weren't good enough[1]. Other drivers just followed it due to cut-and-paste. However, nowadays, most developers are just sticking with the existing common debug infrastructure (either dev_* or pr_* - being the last more used). By using them, Kernel janitors can do a better job, as they may have scripts already prepared to check issues there. I prefer the usage of pr_* macros, as they allow debug messages to be selectively enabled/disabled, if dynamic printk's are enabled. So, a change like that actually improves the debug capability on a given driver. So, IMHO, the better would be to change the patch to use pr_* macros, and follow Joe's suggestions to improve it. Regards, Mauro [1] on other drivers that create their own macros, the usual prefix is the name of the driver (so, on em28xx, it is em28xx_dbg, and so on). In this case, the driver is s5p_tv. So, a macro following the old way should be called as s5p_tv_dbg. Yet, it is takes just a fraction of time to identify them as printk-alike macros, as all those macros are similar. Cheers, 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
[PATCH 0/3] media: Add SH-Mobile RCAR-H2 Lager board support
The following patches add ADV7611/ADV7612 HDMI receiver I2C driver and add RCAR H2 SOC support along with ADV761x output format support to rcar_vin soc_camera driver. These changes are needed for SH-Mobile R8A7790 Lager board video input support. Valentine Barshak (3): media: i2c: Add ADV761X support media: rcar_vin: Add preliminary r8a7790 H2 support media: rcar_vin: Add RGB888_1X24 input format support drivers/media/i2c/Kconfig| 11 + drivers/media/i2c/Makefile |1 + drivers/media/i2c/adv761x.c | 1060 ++ drivers/media/platform/soc_camera/rcar_vin.c | 17 +- include/media/adv761x.h | 28 + 5 files changed, 1114 insertions(+), 3 deletions(-) create mode 100644 drivers/media/i2c/adv761x.c create mode 100644 include/media/adv761x.h -- 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 1/3] media: i2c: Add ADV761X support
This adds ADV7611/ADV7612 Dual Port Xpressview 225 MHz HDMI Receiver support. The code is based on the ADV7604 driver, and ADV7612 patch by Shinobu Uehara shinobu.uehara...@renesas.com Signed-off-by: Valentine Barshak valentine.bars...@cogentembedded.com --- drivers/media/i2c/Kconfig | 11 + drivers/media/i2c/Makefile |1 + drivers/media/i2c/adv761x.c | 1060 +++ include/media/adv761x.h | 28 ++ 4 files changed, 1100 insertions(+) create mode 100644 drivers/media/i2c/adv761x.c create mode 100644 include/media/adv761x.h diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig index d18be19..8eede00 100644 --- a/drivers/media/i2c/Kconfig +++ b/drivers/media/i2c/Kconfig @@ -206,6 +206,17 @@ config VIDEO_ADV7604 To compile this driver as a module, choose M here: the module will be called adv7604. +config VIDEO_ADV761X + tristate Analog Devices ADV761X decoder + depends on VIDEO_V4L2 I2C + ---help--- + Support for the Analog Devices ADV7611/ADV7612 video decoder. + + This is an Analog Devices Dual Port Xpressview HDMI Receiver. + + To compile this driver as a module, choose M here: the + module will be called adv761x. + config VIDEO_ADV7842 tristate Analog Devices ADV7842 decoder depends on VIDEO_V4L2 I2C VIDEO_V4L2_SUBDEV_API diff --git a/drivers/media/i2c/Makefile b/drivers/media/i2c/Makefile index 9f462df..393eb0c 100644 --- a/drivers/media/i2c/Makefile +++ b/drivers/media/i2c/Makefile @@ -26,6 +26,7 @@ obj-$(CONFIG_VIDEO_ADV7183) += adv7183.o obj-$(CONFIG_VIDEO_ADV7343) += adv7343.o obj-$(CONFIG_VIDEO_ADV7393) += adv7393.o obj-$(CONFIG_VIDEO_ADV7604) += adv7604.o +obj-$(CONFIG_VIDEO_ADV761X) += adv761x.o obj-$(CONFIG_VIDEO_ADV7842) += adv7842.o obj-$(CONFIG_VIDEO_AD9389B) += ad9389b.o obj-$(CONFIG_VIDEO_ADV7511) += adv7511.o diff --git a/drivers/media/i2c/adv761x.c b/drivers/media/i2c/adv761x.c new file mode 100644 index 000..bc3dfc3 --- /dev/null +++ b/drivers/media/i2c/adv761x.c @@ -0,0 +1,1060 @@ +/* + * adv761x Analog Devices ADV761X HDMI receiver driver + * + * Copyright (C) 2013 Cogent Embedded, Inc. + * Copyright (C) 2013 Renesas Electronics Corporation + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA. + */ + +#include linux/errno.h +#include linux/i2c.h +#include linux/init.h +#include linux/interrupt.h +#include linux/kernel.h +#include linux/module.h +#include linux/slab.h +#include linux/videodev2.h +#include media/adv761x.h +#include media/v4l2-ctrls.h +#include media/v4l2-device.h +#include media/v4l2-ioctl.h + +#define ADV761X_DRIVER_NAME adv761x + +/* VERT_FILTER_LOCKED and DE_REGEN_FILTER_LOCKED flags */ +#define ADV761X_HDMI_F_LOCKED(v) (((v) 0xa0) == 0xa0) + +/* Maximum supported resolution */ +#define ADV761X_MAX_WIDTH 1920 +#define ADV761X_MAX_HEIGHT 1080 + +static int debug; +module_param(debug, int, 0644); +MODULE_PARM_DESC(debug, debug level (0-2)); + +/* I2C map addresses */ +static u8 i2c_cec = 0x40; +module_param(i2c_cec, byte, 0644); +MODULE_PARM_DESC(i2c_cec, CEC map 7-bit I2C address (default: 0x40)); + +static u8 i2c_inf = 0x3e; +module_param(i2c_inf, byte, 0644); +MODULE_PARM_DESC(i2c_inf, InfoFrame map 7-bit I2C address (default: 0x3e)); + +static u8 i2c_dpll = 0x26; +module_param(i2c_dpll, byte, 0644); +MODULE_PARM_DESC(i2c_dpll, DPLL map 7-bit I2C address (default: 0x20)); + +static u8 i2c_rep = 0x32; +module_param(i2c_rep, byte, 0644); +MODULE_PARM_DESC(i2c_rep, Repeater map 7-bit I2C address (default: 0x32)); + +static u8 i2c_edid = 0x36; +module_param(i2c_edid, byte, 0644); +MODULE_PARM_DESC(i2c_edid, EDID map 7-bit I2C address (default: 0x36)); + +static u8 i2c_hdmi = 0x34; +module_param(i2c_hdmi, byte, 0644); +MODULE_PARM_DESC(i2c_hdmi, HDMI map 7-bit I2C address (default: 0x34)); + +static u8 i2c_cp = 0x22; +module_param(i2c_cp, byte, 0644); +MODULE_PARM_DESC(i2c_cp, CP map 7-bit I2C address (default: 0x22)); + +struct adv761x_color_format { + enum v4l2_mbus_pixelcode code; + enum v4l2_colorspace colorspace; +}; + +/* Supported color format list */ +static const struct adv761x_color_format adv761x_cfmts[] = { + { + .code = V4L2_MBUS_FMT_RGB888_1X24, + .colorspace = V4L2_COLORSPACE_SRGB, + }, +}; + +/* ADV761X
[PATCH 2/3] media: rcar_vin: Add preliminary r8a7790 H2 support
Signed-off-by: Valentine Barshak valentine.bars...@cogentembedded.com --- drivers/media/platform/soc_camera/rcar_vin.c | 5 - 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/drivers/media/platform/soc_camera/rcar_vin.c b/drivers/media/platform/soc_camera/rcar_vin.c index d02a7e0..cf81e02 100644 --- a/drivers/media/platform/soc_camera/rcar_vin.c +++ b/drivers/media/platform/soc_camera/rcar_vin.c @@ -105,6 +105,7 @@ #define VIN_MAX_HEIGHT 2048 enum chip_id { + RCAR_H2, RCAR_H1, RCAR_M1, RCAR_E1, @@ -300,7 +301,8 @@ static int rcar_vin_setup(struct rcar_vin_priv *priv) dmr = 0; break; case V4L2_PIX_FMT_RGB32: - if (priv-chip == RCAR_H1 || priv-chip == RCAR_E1) { + if (priv-chip == RCAR_H2 || priv-chip == RCAR_H1 || + priv-chip == RCAR_E1) { dmr = VNDMR_EXRGB; break; } @@ -1381,6 +1383,7 @@ static struct soc_camera_host_ops rcar_vin_host_ops = { }; static struct platform_device_id rcar_vin_id_table[] = { + { r8a7790-vin, RCAR_H2 }, { r8a7779-vin, RCAR_H1 }, { r8a7778-vin, RCAR_M1 }, { uPD35004-vin, RCAR_E1 }, -- 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 3/3] media: rcar_vin: Add RGB888_1X24 input format support
This adds V4L2_MBUS_FMT_RGB888_1X24 input format support which is used by the ADV7612 chip. Signed-off-by: Valentine Barshak valentine.bars...@cogentembedded.com --- drivers/media/platform/soc_camera/rcar_vin.c | 12 ++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/drivers/media/platform/soc_camera/rcar_vin.c b/drivers/media/platform/soc_camera/rcar_vin.c index cf81e02..0f04cff 100644 --- a/drivers/media/platform/soc_camera/rcar_vin.c +++ b/drivers/media/platform/soc_camera/rcar_vin.c @@ -68,6 +68,7 @@ #define VNMC_INF_YUV8_BT656(0 16) #define VNMC_INF_YUV8_BT601(1 16) #define VNMC_INF_YUV16 (5 16) +#define VNMC_INF_RGB888(6 16) #define VNMC_VUP (1 10) #define VNMC_IM_ODD(0 3) #define VNMC_IM_ODD_EVEN (1 3) @@ -235,7 +236,7 @@ static int rcar_vin_setup(struct rcar_vin_priv *priv) struct soc_camera_device *icd = priv-ici.icd; struct rcar_vin_cam *cam = icd-host_priv; u32 vnmc, dmr, interrupts; - bool progressive = false, output_is_yuv = false; + bool progressive = false, output_is_yuv = false, input_is_yuv = false; switch (priv-field) { case V4L2_FIELD_TOP: @@ -269,11 +270,17 @@ static int rcar_vin_setup(struct rcar_vin_priv *priv) case V4L2_MBUS_FMT_YUYV8_1X16: /* BT.601/BT.1358 16bit YCbCr422 */ vnmc |= VNMC_INF_YUV16; + input_is_yuv = true; break; case V4L2_MBUS_FMT_YUYV8_2X8: /* BT.656 8bit YCbCr422 or BT.601 8bit YCbCr422 */ vnmc |= priv-pdata-flags RCAR_VIN_BT656 ? VNMC_INF_YUV8_BT656 : VNMC_INF_YUV8_BT601; + input_is_yuv = true; + break; + case V4L2_MBUS_FMT_RGB888_1X24: + vnmc |= VNMC_INF_RGB888; + break; default: break; } @@ -316,7 +323,7 @@ static int rcar_vin_setup(struct rcar_vin_priv *priv) vnmc |= VNMC_VUP; /* If input and output use the same colorspace, use bypass mode */ - if (output_is_yuv) + if (input_is_yuv == output_is_yuv) vnmc |= VNMC_BPS; /* progressive or interlaced mode */ @@ -1002,6 +1009,7 @@ static int rcar_vin_get_formats(struct soc_camera_device *icd, unsigned int idx, switch (code) { case V4L2_MBUS_FMT_YUYV8_1X16: case V4L2_MBUS_FMT_YUYV8_2X8: + case V4L2_MBUS_FMT_RGB888_1X24: if (cam-extra_fmt) break; -- 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 1/3] media: i2c: Add ADV761X support
Hi Valentine, On Tue 24 September 2013 15:38:34 Valentine Barshak wrote: This adds ADV7611/ADV7612 Dual Port Xpressview 225 MHz HDMI Receiver support. The code is based on the ADV7604 driver, and ADV7612 patch by Shinobu Uehara shinobu.uehara...@renesas.com Thanks for the patch! I have a few review comments below: Signed-off-by: Valentine Barshak valentine.bars...@cogentembedded.com --- drivers/media/i2c/Kconfig | 11 + drivers/media/i2c/Makefile |1 + drivers/media/i2c/adv761x.c | 1060 +++ include/media/adv761x.h | 28 ++ 4 files changed, 1100 insertions(+) create mode 100644 drivers/media/i2c/adv761x.c create mode 100644 include/media/adv761x.h diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig index d18be19..8eede00 100644 --- a/drivers/media/i2c/Kconfig +++ b/drivers/media/i2c/Kconfig @@ -206,6 +206,17 @@ config VIDEO_ADV7604 To compile this driver as a module, choose M here: the module will be called adv7604. +config VIDEO_ADV761X + tristate Analog Devices ADV761X decoder + depends on VIDEO_V4L2 I2C + ---help--- + Support for the Analog Devices ADV7611/ADV7612 video decoder. + + This is an Analog Devices Dual Port Xpressview HDMI Receiver. + + To compile this driver as a module, choose M here: the + module will be called adv761x. + config VIDEO_ADV7842 tristate Analog Devices ADV7842 decoder depends on VIDEO_V4L2 I2C VIDEO_V4L2_SUBDEV_API diff --git a/drivers/media/i2c/Makefile b/drivers/media/i2c/Makefile index 9f462df..393eb0c 100644 --- a/drivers/media/i2c/Makefile +++ b/drivers/media/i2c/Makefile @@ -26,6 +26,7 @@ obj-$(CONFIG_VIDEO_ADV7183) += adv7183.o obj-$(CONFIG_VIDEO_ADV7343) += adv7343.o obj-$(CONFIG_VIDEO_ADV7393) += adv7393.o obj-$(CONFIG_VIDEO_ADV7604) += adv7604.o +obj-$(CONFIG_VIDEO_ADV761X) += adv761x.o obj-$(CONFIG_VIDEO_ADV7842) += adv7842.o obj-$(CONFIG_VIDEO_AD9389B) += ad9389b.o obj-$(CONFIG_VIDEO_ADV7511) += adv7511.o diff --git a/drivers/media/i2c/adv761x.c b/drivers/media/i2c/adv761x.c new file mode 100644 index 000..bc3dfc3 --- /dev/null +++ b/drivers/media/i2c/adv761x.c @@ -0,0 +1,1060 @@ +/* + * adv761x Analog Devices ADV761X HDMI receiver driver + * + * Copyright (C) 2013 Cogent Embedded, Inc. + * Copyright (C) 2013 Renesas Electronics Corporation + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA. + */ + +#include linux/errno.h +#include linux/i2c.h +#include linux/init.h +#include linux/interrupt.h +#include linux/kernel.h +#include linux/module.h +#include linux/slab.h +#include linux/videodev2.h +#include media/adv761x.h +#include media/v4l2-ctrls.h +#include media/v4l2-device.h +#include media/v4l2-ioctl.h + +#define ADV761X_DRIVER_NAME adv761x + +/* VERT_FILTER_LOCKED and DE_REGEN_FILTER_LOCKED flags */ +#define ADV761X_HDMI_F_LOCKED(v) (((v) 0xa0) == 0xa0) + +/* Maximum supported resolution */ +#define ADV761X_MAX_WIDTH1920 +#define ADV761X_MAX_HEIGHT 1080 + +static int debug; +module_param(debug, int, 0644); +MODULE_PARM_DESC(debug, debug level (0-2)); + +/* I2C map addresses */ +static u8 i2c_cec = 0x40; +module_param(i2c_cec, byte, 0644); +MODULE_PARM_DESC(i2c_cec, CEC map 7-bit I2C address (default: 0x40)); + +static u8 i2c_inf = 0x3e; +module_param(i2c_inf, byte, 0644); +MODULE_PARM_DESC(i2c_inf, InfoFrame map 7-bit I2C address (default: 0x3e)); + +static u8 i2c_dpll = 0x26; +module_param(i2c_dpll, byte, 0644); +MODULE_PARM_DESC(i2c_dpll, DPLL map 7-bit I2C address (default: 0x20)); + +static u8 i2c_rep = 0x32; +module_param(i2c_rep, byte, 0644); +MODULE_PARM_DESC(i2c_rep, Repeater map 7-bit I2C address (default: 0x32)); + +static u8 i2c_edid = 0x36; +module_param(i2c_edid, byte, 0644); +MODULE_PARM_DESC(i2c_edid, EDID map 7-bit I2C address (default: 0x36)); + +static u8 i2c_hdmi = 0x34; +module_param(i2c_hdmi, byte, 0644); +MODULE_PARM_DESC(i2c_hdmi, HDMI map 7-bit I2C address (default: 0x34)); + +static u8 i2c_cp = 0x22; +module_param(i2c_cp, byte, 0644); +MODULE_PARM_DESC(i2c_cp, CP map 7-bit I2C address (default: 0x22)); Don't use module options for the i2c addresses. Instead use platform_data. Boards may have multiple adv761x devices behind e.g. a i2c
[RFC PATCH 0/4] CDFv3: MIPI DSI bus implementation
Hi Laurent, This is my MIPI DSI bus implementation. The patchset contains also two drivers: - DSI bus master driver for Exynos, - DSI slave driver for s6e8aa0 panel family. All code has been tested on real device - Exynos 4210 based 'Trats'. This is not final version, there are still some things to do. DSI bus code is based on mipi-dbi-bus, with few major changes. 1. I have replaced three DBI opses: - write_command, - write_data, - read_data with one op 'transfer', this way it better fits to MIPI DSI standard, I wonder if this cannot be adapted to DBI also. The only things which bothers me is number of arguments, maybe struct should be used instead. I have added DCS helpers functions which use 'transfer' op: - mipi_dsi_dcs_read - mipi_dsi_dcs_write and two macros which allow to pass variable number of bytes as arguments, example usage in panel driver code: - mipi_dsi_dcs_write_seq - mipi_dsi_dcs_write_static_seq I have added also following opses: - set_stream - to enable/disable streaming on DSI master, - set_power - I have temporarily dropped idea of using runtime_pm due to compliacted power on sequence of panel/dsi, which would probably require different op anyway. struct mipi_dsi_device contains videomode and mipi_dsi_interface_params fields. Those fields are read by opses, I wonder if it would not be better to pass them directly to opses as arguments. TODO: - helpers for non-DT drivers, - minor power management issues, - better error handling - ... Regards Andrzej Andrzej Hajda (4): mipi-dsi-bus: add MIPI DSI bus support mipi-dsi-exynos: add driver panel-s6e8aa0: add driver ARM: dts: exynos4210-trats: add panel and dsi nodes arch/arm/boot/dts/exynos4210-trats.dts | 54 ++ drivers/video/display/Kconfig | 14 + drivers/video/display/Makefile |3 + drivers/video/display/mipi-dsi-bus.c| 332 drivers/video/display/mipi-dsi-exynos.c | 1310 +++ drivers/video/display/panel-s6e8aa0.c | 1286 ++ include/video/display.h |3 + include/video/mipi-dsi-bus.h| 144 include/video/mipi-dsi-exynos.h | 41 + include/video/panel-s6e8aa0.h | 42 + 10 files changed, 3229 insertions(+) create mode 100644 drivers/video/display/mipi-dsi-bus.c create mode 100644 drivers/video/display/mipi-dsi-exynos.c create mode 100644 drivers/video/display/panel-s6e8aa0.c create mode 100644 include/video/mipi-dsi-bus.h create mode 100644 include/video/mipi-dsi-exynos.h create mode 100644 include/video/panel-s6e8aa0.h -- 1.8.1.2 -- 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 2/4] mipi-dsi-exynos: add driver
This patch adds mipi-dsi-bus master driver for Exynos chipset family. Signed-off-by: Tomasz Figa t.f...@samsung.com Signed-off-by: Donghwa Lee dh09@samsung.com Signed-off-by: Sylwester Nawrocki s.nawro...@samsung.com Signed-off-by: Andrzej Hajda a.ha...@samsung.com Signed-off-by: Kyungmin Park kyungmin.p...@samsung.com --- drivers/video/display/Kconfig |4 + drivers/video/display/Makefile |1 + drivers/video/display/mipi-dsi-exynos.c | 1310 +++ include/video/mipi-dsi-exynos.h | 41 + 4 files changed, 1356 insertions(+) create mode 100644 drivers/video/display/mipi-dsi-exynos.c create mode 100644 include/video/mipi-dsi-exynos.h diff --git a/drivers/video/display/Kconfig b/drivers/video/display/Kconfig index 619b05d..0a1e90b 100644 --- a/drivers/video/display/Kconfig +++ b/drivers/video/display/Kconfig @@ -24,6 +24,10 @@ config DISPLAY_MIPI_DSI tristate default n +config DISPLAY_MIPI_DSI_EXYNOS + select DISPLAY_MIPI_DSI + tristate Samsung SoC MIPI DSI Master + config DISPLAY_PANEL_DPI tristate DPI (Parallel) Display Panels ---help--- diff --git a/drivers/video/display/Makefile b/drivers/video/display/Makefile index b323fd4..2fd84f5 100644 --- a/drivers/video/display/Makefile +++ b/drivers/video/display/Makefile @@ -4,6 +4,7 @@ obj-$(CONFIG_DISPLAY_CORE) += display.o obj-$(CONFIG_DISPLAY_CONNECTOR_VGA)+= con-vga.o obj-$(CONFIG_DISPLAY_MIPI_DBI) += mipi-dbi-bus.o obj-$(CONFIG_DISPLAY_MIPI_DSI) += mipi-dsi-bus.o +obj-$(CONFIG_DISPLAY_MIPI_DSI_EXYNOS) += mipi-dsi-exynos.o obj-$(CONFIG_DISPLAY_PANEL_DPI)+= panel-dpi.o obj-$(CONFIG_DISPLAY_PANEL_R61505) += panel-r61505.o obj-$(CONFIG_DISPLAY_PANEL_R61517) += panel-r61517.o diff --git a/drivers/video/display/mipi-dsi-exynos.c b/drivers/video/display/mipi-dsi-exynos.c new file mode 100644 index 000..e094744 --- /dev/null +++ b/drivers/video/display/mipi-dsi-exynos.c @@ -0,0 +1,1310 @@ +/* + * Samsung SoC MIPI DSI Master driver. + * + * Copyright (c) 2012 Samsung Electronics Co., Ltd + * + * Contacts: Tomasz Figa t.f...@samsung.com + * + * 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 linux/clk.h +#include linux/completion.h +#include linux/delay.h +#include linux/errno.h +#include linux/fb.h +#include linux/interrupt.h +#include linux/io.h +#include linux/irq.h +#include linux/kernel.h +#include linux/list.h +#include linux/memory.h +#include linux/mm.h +#include linux/module.h +#include linux/of.h +#include linux/phy/phy.h +#include linux/platform_device.h +#include linux/pm_runtime.h +#include linux/regulator/consumer.h +#include linux/spinlock.h +#include linux/wait.h + +#include video/mipi_display.h +#include video/mipi-dsi-bus.h +#include video/mipi-dsi-exynos.h +#include video/videomode.h + +#define DSIM_STATUS_REG0x0 /* Status register */ +#define DSIM_SWRST_REG 0x4 /* Software reset register */ +#define DSIM_CLKCTRL_REG 0x8 /* Clock control register */ +#define DSIM_TIMEOUT_REG 0xc /* Time out register */ +#define DSIM_CONFIG_REG0x10/* Configuration register */ +#define DSIM_ESCMODE_REG 0x14/* Escape mode register */ + +/* Main display image resolution register */ +#define DSIM_MDRESOL_REG 0x18 +#define DSIM_MVPORCH_REG 0x1c/* Main display Vporch register */ +#define DSIM_MHPORCH_REG 0x20/* Main display Hporch register */ +#define DSIM_MSYNC_REG 0x24/* Main display sync area register */ + +/* Sub display image resolution register */ +#define DSIM_SDRESOL_REG 0x28 +#define DSIM_INTSRC_REG0x2c/* Interrupt source register */ +#define DSIM_INTMSK_REG0x30/* Interrupt mask register */ +#define DSIM_PKTHDR_REG0x34/* Packet Header FIFO register */ +#define DSIM_PAYLOAD_REG 0x38/* Payload FIFO register */ +#define DSIM_RXFIFO_REG0x3c/* Read FIFO register */ +#define DSIM_FIFOTHLD_REG 0x40/* FIFO threshold level register */ +#define DSIM_FIFOCTRL_REG 0x44/* FIFO status and control register */ + +/* FIFO memory AC characteristic register */ +#define DSIM_PLLCTRL_REG 0x4c/* PLL control register */ +#define DSIM_PLLTMR_REG0x50/* PLL timer register */ +#define DSIM_PHYACCHR_REG 0x54/* D-PHY AC characteristic register */ +#define DSIM_PHYACCHR1_REG 0x58/* D-PHY AC characteristic register1 */ + +/* DSIM_STATUS */ +#define DSIM_STOP_STATE_DAT(x) (((x) 0xf) 0) +#define DSIM_STOP_STATE_CLK(1 8) +#define DSIM_TX_READY_HS_CLK (1 10) +#define DSIM_PLL_STABLE
[RFC PATCH 3/4] panel-s6e8aa0: add driver
The patch adds mipi-dsi-bus slave driver for s6e8aa0 familiy panels. Signed-off-by: Donghwa Lee dh09@samsung.com Signed-off-by: Inki Dae inki@samsung.com Signed-off-by: Joongmock Shin jmock.s...@samsung.com Signed-off-by: Eunchul Kim chulspro@samsung.com Signed-off-by: Tomasz Figa t.f...@samsung.com Signed-off-by: Andrzej Hajda a.ha...@samsung.com Signed-off-by: Kyungmin Park kyungmin.p...@samsung.com --- drivers/video/display/Kconfig |6 + drivers/video/display/Makefile|1 + drivers/video/display/panel-s6e8aa0.c | 1286 + include/video/panel-s6e8aa0.h | 42 ++ 4 files changed, 1335 insertions(+) create mode 100644 drivers/video/display/panel-s6e8aa0.c create mode 100644 include/video/panel-s6e8aa0.h diff --git a/drivers/video/display/Kconfig b/drivers/video/display/Kconfig index 0a1e90b..9a9b7e9 100644 --- a/drivers/video/display/Kconfig +++ b/drivers/video/display/Kconfig @@ -67,4 +67,10 @@ config DISPLAY_VGA_DAC If you are in doubt, say N. To compile this driver as a module, choose M here: the module will be called vga-dac. +config DISPLAY_PANEL_S6E8AA0 + tristate S6E8AA0 DSI video mode panel + select BACKLIGHT_CLASS_DEVICE + select DISPLAY_MIPI_DSI + select VIDEOMODE_HELPERS + endif # DISPLAY_CORE diff --git a/drivers/video/display/Makefile b/drivers/video/display/Makefile index 2fd84f5..45225e1 100644 --- a/drivers/video/display/Makefile +++ b/drivers/video/display/Makefile @@ -9,3 +9,4 @@ obj-$(CONFIG_DISPLAY_PANEL_DPI) += panel-dpi.o obj-$(CONFIG_DISPLAY_PANEL_R61505) += panel-r61505.o obj-$(CONFIG_DISPLAY_PANEL_R61517) += panel-r61517.o obj-$(CONFIG_DISPLAY_VGA_DAC) += vga-dac.o +obj-$(CONFIG_DISPLAY_PANEL_S6E8AA0)+= panel-s6e8aa0.o diff --git a/drivers/video/display/panel-s6e8aa0.c b/drivers/video/display/panel-s6e8aa0.c new file mode 100644 index 000..99246da --- /dev/null +++ b/drivers/video/display/panel-s6e8aa0.c @@ -0,0 +1,1286 @@ +/* linux/drivers/video/s6e8aa0.c + * + * MIPI-DSI based s6e8aa0 AMOLED lcd 5.3 inch panel driver. + * + * Inki Dae, inki@samsung.com + * Donghwa Lee, dh09@samsung.com + * Joongmock Shin jmock.s...@samsung.com + * Eunchul Kim chulspro@samsung.com + * Tomasz Figa t.f...@samsung.com + * + * 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 linux/module.h +#include linux/kernel.h + +#include linux/backlight.h +#include linux/ctype.h +#include linux/delay.h +#include linux/errno.h +#include linux/fb.h +#include linux/gpio.h +#include linux/lcd.h +#include linux/mutex.h +#include linux/of.h +#include linux/of_gpio.h +#include linux/platform_device.h +#include linux/regulator/consumer.h + +#include video/mipi_display.h +#include video/mipi-dsi-bus.h +#include video/of_videomode.h +#include video/panel-s6e8aa0.h + +#define LDI_MTP_LENGTH 24 +#define GAMMA_LEVEL_NUM25 +#define GAMMA_TABLE_LEN26 +#define S6E8AA0_PANEL_COND_LEN 39 + +/* 1 */ +#define PANELCTL_SS_MASK (1 5) +#define PANELCTL_SS_1_800 (0 5) +#define PANELCTL_SS_800_1 (1 5) +#define PANELCTL_GTCON_MASK(7 2) +#define PANELCTL_GTCON_110 (6 2) +#define PANELCTL_GTCON_111 (7 2) +/* LTPS */ +/* 30 */ +#define PANELCTL_CLK1_CON_MASK (7 3) +#define PANELCTL_CLK1_000 (0 3) +#define PANELCTL_CLK1_001 (1 3) +#define PANELCTL_CLK2_CON_MASK (7 0) +#define PANELCTL_CLK2_000 (0 0) +#define PANELCTL_CLK2_001 (1 0) +/* 31 */ +#define PANELCTL_INT1_CON_MASK (7 3) +#define PANELCTL_INT1_000 (0 3) +#define PANELCTL_INT1_001 (1 3) +#define PANELCTL_INT2_CON_MASK (7 0) +#define PANELCTL_INT2_000 (0 0) +#define PANELCTL_INT2_001 (1 0) +/* 32 */ +#define PANELCTL_BICTL_CON_MASK(7 3) +#define PANELCTL_BICTL_000 (0 3) +#define PANELCTL_BICTL_001 (1 3) +#define PANELCTL_BICTLB_CON_MASK (7 0) +#define PANELCTL_BICTLB_000(0 0) +#define PANELCTL_BICTLB_001(1 0) +/* 36 */ +#define PANELCTL_EM_CLK1_CON_MASK (7 3) +#define PANELCTL_EM_CLK1_110 (6 3) +#define PANELCTL_EM_CLK1_111 (7 3) +#define PANELCTL_EM_CLK1B_CON_MASK (7 0) +#define PANELCTL_EM_CLK1B_110 (6 0) +#define PANELCTL_EM_CLK1B_111 (7 0) +/* 37 */ +#define PANELCTL_EM_CLK2_CON_MASK (7 3) +#define PANELCTL_EM_CLK2_110 (6 3) +#define PANELCTL_EM_CLK2_111 (7 3) +#define PANELCTL_EM_CLK2B_CON_MASK (7 0) +#define PANELCTL_EM_CLK2B_110 (6 0) +#define
[RFC PATCH 1/4] mipi-dsi-bus: add MIPI DSI bus support
MIPI DSI is a high-speed serial interface to transmit data from/to host to display module. Signed-off-by: Andrzej Hajda a.ha...@samsung.com Signed-off-by: Kyungmin Park kyungmin.p...@samsung.com --- drivers/video/display/Kconfig| 4 + drivers/video/display/Makefile | 1 + drivers/video/display/mipi-dsi-bus.c | 332 +++ include/video/display.h | 3 + include/video/mipi-dsi-bus.h | 144 +++ 5 files changed, 484 insertions(+) create mode 100644 drivers/video/display/mipi-dsi-bus.c create mode 100644 include/video/mipi-dsi-bus.h diff --git a/drivers/video/display/Kconfig b/drivers/video/display/Kconfig index 9b482a8..619b05d 100644 --- a/drivers/video/display/Kconfig +++ b/drivers/video/display/Kconfig @@ -20,6 +20,10 @@ config DISPLAY_MIPI_DBI tristate default n +config DISPLAY_MIPI_DSI + tristate + default n + config DISPLAY_PANEL_DPI tristate DPI (Parallel) Display Panels ---help--- diff --git a/drivers/video/display/Makefile b/drivers/video/display/Makefile index d03c64a..b323fd4 100644 --- a/drivers/video/display/Makefile +++ b/drivers/video/display/Makefile @@ -3,6 +3,7 @@ display-y := display-core.o \ obj-$(CONFIG_DISPLAY_CORE) += display.o obj-$(CONFIG_DISPLAY_CONNECTOR_VGA)+= con-vga.o obj-$(CONFIG_DISPLAY_MIPI_DBI) += mipi-dbi-bus.o +obj-$(CONFIG_DISPLAY_MIPI_DSI) += mipi-dsi-bus.o obj-$(CONFIG_DISPLAY_PANEL_DPI)+= panel-dpi.o obj-$(CONFIG_DISPLAY_PANEL_R61505) += panel-r61505.o obj-$(CONFIG_DISPLAY_PANEL_R61517) += panel-r61517.o diff --git a/drivers/video/display/mipi-dsi-bus.c b/drivers/video/display/mipi-dsi-bus.c new file mode 100644 index 000..a194d92 --- /dev/null +++ b/drivers/video/display/mipi-dsi-bus.c @@ -0,0 +1,332 @@ +/* + * MIPI DSI Bus + * + * Copyright (C) 2012, Samsung Electronics, Co., Ltd. + * Andrzej Hajda a.ha...@samsung.com + * + * 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 linux/device.h +#include linux/export.h +#include linux/kernel.h +#include linux/list.h +#include linux/of_device.h +#include linux/module.h +#include linux/mutex.h +#include linux/slab.h +#include linux/pm.h +#include linux/pm_runtime.h +#include video/mipi_display.h +#include video/mipi-dsi-bus.h + +/* - + * Bus operations + */ + +int mipi_dsi_set_power(struct mipi_dsi_device *dev, bool on) +{ + const struct mipi_dsi_bus_ops *ops = dev-bus-ops; + + if (!ops-set_power) + return 0; + + return ops-set_power(dev-bus, dev, on); +} +EXPORT_SYMBOL_GPL(mipi_dsi_set_power); + +int mipi_dsi_set_stream(struct mipi_dsi_device *dev, bool on) +{ + const struct mipi_dsi_bus_ops *ops = dev-bus-ops; + + if (!ops-set_stream) + return 0; + + return ops-set_stream(dev-bus, dev, on); +} +EXPORT_SYMBOL_GPL(mipi_dsi_set_stream); + +int mipi_dsi_dcs_write(struct mipi_dsi_device *dev, int channel, const u8 *data, + size_t len) +{ + const struct mipi_dsi_bus_ops *ops = dev-bus-ops; + u8 type = channel 6; + + if (!ops-transfer) + return -EINVAL; + + switch (len) { + case 0: + return -EINVAL; + case 1: + type |= MIPI_DSI_DCS_SHORT_WRITE; + break; + case 2: + type |= MIPI_DSI_DCS_SHORT_WRITE_PARAM; + break; + default: + type |= MIPI_DSI_DCS_LONG_WRITE; + } + + return ops-transfer(dev-bus, dev, type, data, len, NULL, 0); +} +EXPORT_SYMBOL_GPL(mipi_dsi_dcs_write); + +int mipi_dsi_dcs_read(struct mipi_dsi_device *dev, int channel, u8 cmd, + u8 *data, size_t len) +{ + const struct mipi_dsi_bus_ops *ops = dev-bus-ops; + u8 type = MIPI_DSI_DCS_READ | (channel 6); + + if (!ops-transfer) + return -EINVAL; + + return ops-transfer(dev-bus, dev, type, cmd, 1, data, len); +} +EXPORT_SYMBOL_GPL(mipi_dsi_dcs_read); + +/* - + * Bus type + */ + +static const struct mipi_dsi_device_id * +mipi_dsi_match_id(const struct mipi_dsi_device_id *id, + struct mipi_dsi_device *dev) +{ + while (id-name[0]) { + if (strcmp(dev-name, id-name) == 0) { + dev-id_entry = id; + return id; + } + id++; + } + return NULL; +} + +static int mipi_dsi_match(struct device *_dev, struct device_driver *_drv) +{ + struct mipi_dsi_device *dev =
[RFC PATCH 4/4] ARM: dts: exynos4210-trats: add panel and dsi nodes
The patch adds mipi-dsi-exynos bus master node and s6e8aa0 panel subnode to trats device. Signed-off-by: Andrzej Hajda a.ha...@samsung.com Signed-off-by: Kyungmin Park kyungmin.p...@samsung.com Conflicts: arch/arm/boot/dts/exynos4210-trats.dts --- arch/arm/boot/dts/exynos4210-trats.dts | 54 ++ 1 file changed, 54 insertions(+) diff --git a/arch/arm/boot/dts/exynos4210-trats.dts b/arch/arm/boot/dts/exynos4210-trats.dts index 94eebff..6f1a034 100644 --- a/arch/arm/boot/dts/exynos4210-trats.dts +++ b/arch/arm/boot/dts/exynos4210-trats.dts @@ -301,4 +301,58 @@ clock-frequency = 2400; }; }; + + dsi_0: dsi@11C8 { + samsung,pll-stable-time = 500; + samsung,stop-holding-count = 0x7ff; + samsung,bta-timeout = 0xff; + samsung,rx-timeout = 0x; + samsung,pll-clk-freq = 2400; + samsung,cmd-allow = 0xf; + vdd11-supply = vusb_reg; + vdd18-supply = vmipi_reg; + status = okay; + + fimd0_lcd: panel { + compatible = samsung,s6e8aa0; + reset-gpio = gpy4 5 0; + power-on-delay= 50; + reset-delay = 100; + init-delay = 100; + vdd3-supply = vcclcd_reg; + vci-supply = vlcd_reg; + video-source = dsi_0; + flip-horizontal; + flip-vertical; + samsung,panel-width-mm = 58; + samsung,panel-height-mm = 103; + + display-timings { + native-mode = timing0; + + timing0: timing-0 { + clock-frequency = 0; + hactive = 720; + vactive = 1280; + hfront-porch = 5; + hback-porch = 5; + hsync-len = 5; + vfront-porch = 13; + vback-porch = 1; + vsync-len = 2; + }; + }; + }; + }; + + fimd@11c0 { + samsung,fimd-display = fimd0_lcd; + samsung,fimd-vidout-rgb; + samsung,fimd-inv-vclk; + samsung,fimd-frame-rate = 60; + samsung,default-window = 3; + samsung,fimd-win-bpp = 32; + status = okay; + }; + }; -- 1.8.1.2 -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5 1/4] media: s5p-tv: Replace mxr_ macro by default dev_
Hi Tomasz, On Tuesday, September 24, 2013 02:52:44 PM Tomasz Stanislawski wrote: On 09/23/2013 07:44 PM, Joe Perches wrote: On Mon, 2013-09-23 at 17:48 +0200, Bartlomiej Zolnierkiewicz wrote: On Monday, September 23, 2013 04:50:01 PM Tomasz Stanislawski wrote: May I ask what is the rationale for this patch? To reduce a few lines of code? This patch makes source code more generic-like and easier to follow (mxd_r* macros currently only obfuscate the code and make them harder to read for everybody, maybe besides the original driver author ;). Removal of few superfluous lines of code is just a bonus. I don't see any significant issue with this change. Using generic mechanisms is good. Hi Joe, Sorry for flaming but please let me explain reasons of my opposition to this patch. 1. It is true that there was no change in mixer messages for 2.5 year in MAINLINE. But sometimes I used modification of mxr_ macros while testing the driver. Therefore those macros are useful for me. For debug you can also trivially do this with dev_*, just #undef them in your driver and define your versions. 2. The other problem with this patch is its high 'conflictness' during merging. Unfortunately, sometimes I have to use s5p-tv on platform and configuration that is only supported in older versions of the kernel + some integration patches. The s5p-tv differs from mainline version in those kernels. Therefore I would need to keep two versions of patches, one for old and another one for new kernel. You would need to do it or not, depending on the actual change. Anyway in the reality there is practically no development happening on this driver. During two and half year you only did two small fixes to the mixer driver (BTW your other drivers from s5p-tv directory are not using any custom macros): 3c44efd [media] v4l: s5p-tv: mixer: support for dmabuf exporting fa77521 [media] v4l: s5p-tv: mixer: support for dmabuf importing Or backport the 'cleanup patch' and all experimental patches above it. The cost to backport it if/when needed should be pretty small, it is not a big change: 6 files changed, 78 insertions(+), 91 deletions(-) 3. As I understand the coding guidelines asked to use dev_* to ensure that all error messages have information about the device. There is no change in format of errors after this patch. So they do not change anything from userland point of view. Which is actually a good thing (same functionality, less code). 4. I looked for other files where macro for dev_err is used. I tried following shell command on v3.12-rc2. git grep -A1 _err( | grep -A1 '#define' | grep -B1 dev_err then processing results using grep -v ^-- | cut -d: -f 1 | sort -u | wc produced 55 files. Among them, the files below makes use of a macro that is directly expanded to dev_err(dev_ptr, fmt, ...) without any changes in format. drivers/firewire/ohci.c drivers/gpu/drm/i2c/ch7006_priv.h drivers/gpu/drm/i2c/sil164_drv.c drivers/infiniband/hw/mthca/mthca_dev.h drivers/infiniband/hw/qib/qib.h drivers/media/platform/marvell-ccic/cafe-driver.c drivers/media/platform/marvell-ccic/mcam-core.c drivers/media/platform/s5p-tv/mixer.h drivers/media/platform/via-camera.c drivers/mtd/devices/docg3.h drivers/net/ethernet/broadcom/bgmac.h drivers/net/ethernet/chelsio/cxgb3/common.h drivers/net/ethernet/intel/e1000/e1000.h drivers/net/ethernet/intel/ixgbe/ixgbe_common.h drivers/net/ethernet/mellanox/mlx4/mlx4.h drivers/net/wireless/iwlegacy/common.h drivers/pci/hotplug/pciehp.h drivers/pci/hotplug/shpchp.h drivers/remoteproc/ste_modem_rproc.c drivers/scsi/csiostor/csio_hw.h drivers/staging/fwserial/fwserial.c drivers/usb/atm/usbatm.h drivers/usb/host/ehci.h drivers/usb/host/fhci.h drivers/usb/host/fotg210-hcd.c drivers/usb/host/fusbh200-hcd.c drivers/usb/host/ohci.h drivers/usb/host/oxu210hp-hcd.c drivers/usb/host/xhci.h include/linux/hid.h include/net/cfg80211.h Other files makes only cosmetic changes to format, so they might still be worth to be 'demacronized'. So I think we can consider that macros wrapping dev_* is still a widely used technique so I ask for a good reason before changing the driver. If one still would like to continue a 'dev_* cleanup crusade' then I kindly ask to create a big patchset that fixes all over mentioned files. If most of their maintainers accepts the patches I promise to accept it in s5p-tv. OK. Currently, due to mentioned reason the patch is not a cleanup-up for me. And since I am still a maintainer of this god-forgotten driver I am going NACK this patch because it makes my work more difficult and because this patch provides only (if any) relative aesthetic gain. I won't argue about this anymore because I find the whole discussion a bit silly. The change itself is obvious, trivial and cost to either port new patches over it or backport it to older private trees should be very small (as I sit next to
Re: [PATCH] media: i2c: adv7343: fix the DT binding properties
On Tue, Sep 24, 2013 at 11:44:57AM +0200, Laurent Pinchart wrote: On Monday 23 September 2013 15:33:10 Stephen Warren wrote: So I think you want to make the supply properties mandatory in DT (since some form of supply is mandatory in HW), yet make the driver support broken DTs which don't have those properties, by error-checking the return value from regulator_get(). You might want to put a note into DT saying that a previous version of the binding didn't require those supply properties, so they may be missing from older DTs. Are there such devices in the wild ? From v3.12 on the kernel will stub in a dummy supply if one isn't found when using DT but I wouldn't mention that in the bindings since it's very much fixing up broken DTs rather than a good idea. signature.asc Description: Digital signature
Re: [PATCH 1/3] media: i2c: Add ADV761X support
Hi Valentine, On Tue, 24 Sep 2013, Valentine Barshak wrote: This adds ADV7611/ADV7612 Dual Port Xpressview 225 MHz HDMI Receiver support. The code is based on the ADV7604 driver, and ADV7612 patch by Shinobu Uehara shinobu.uehara...@renesas.com Signed-off-by: Valentine Barshak valentine.bars...@cogentembedded.com IIRC, Laurent is reviewing all new media I2C drivers, I added him to cc. A couple of minor comments from me too, while at it. --- drivers/media/i2c/Kconfig | 11 + drivers/media/i2c/Makefile |1 + drivers/media/i2c/adv761x.c | 1060 +++ include/media/adv761x.h | 28 ++ 4 files changed, 1100 insertions(+) create mode 100644 drivers/media/i2c/adv761x.c create mode 100644 include/media/adv761x.h diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig index d18be19..8eede00 100644 --- a/drivers/media/i2c/Kconfig +++ b/drivers/media/i2c/Kconfig @@ -206,6 +206,17 @@ config VIDEO_ADV7604 To compile this driver as a module, choose M here: the module will be called adv7604. +config VIDEO_ADV761X + tristate Analog Devices ADV761X decoder + depends on VIDEO_V4L2 I2C + ---help--- + Support for the Analog Devices ADV7611/ADV7612 video decoder. + + This is an Analog Devices Dual Port Xpressview HDMI Receiver. Are you sure this driver can handle all adv761x devices? One of the differences even between 7611 and 7612 is, that only 7612 is dual-port, 7611 is single-port. What about 7619? + + To compile this driver as a module, choose M here: the + module will be called adv761x. + config VIDEO_ADV7842 tristate Analog Devices ADV7842 decoder depends on VIDEO_V4L2 I2C VIDEO_V4L2_SUBDEV_API diff --git a/drivers/media/i2c/Makefile b/drivers/media/i2c/Makefile index 9f462df..393eb0c 100644 --- a/drivers/media/i2c/Makefile +++ b/drivers/media/i2c/Makefile @@ -26,6 +26,7 @@ obj-$(CONFIG_VIDEO_ADV7183) += adv7183.o obj-$(CONFIG_VIDEO_ADV7343) += adv7343.o obj-$(CONFIG_VIDEO_ADV7393) += adv7393.o obj-$(CONFIG_VIDEO_ADV7604) += adv7604.o +obj-$(CONFIG_VIDEO_ADV761X) += adv761x.o obj-$(CONFIG_VIDEO_ADV7842) += adv7842.o obj-$(CONFIG_VIDEO_AD9389B) += ad9389b.o obj-$(CONFIG_VIDEO_ADV7511) += adv7511.o diff --git a/drivers/media/i2c/adv761x.c b/drivers/media/i2c/adv761x.c new file mode 100644 index 000..bc3dfc3 --- /dev/null +++ b/drivers/media/i2c/adv761x.c @@ -0,0 +1,1060 @@ +/* + * adv761x Analog Devices ADV761X HDMI receiver driver + * + * Copyright (C) 2013 Cogent Embedded, Inc. + * Copyright (C) 2013 Renesas Electronics Corporation + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA. + */ + +#include linux/errno.h +#include linux/i2c.h +#include linux/init.h +#include linux/interrupt.h +#include linux/kernel.h +#include linux/module.h +#include linux/slab.h +#include linux/videodev2.h +#include media/adv761x.h +#include media/v4l2-ctrls.h +#include media/v4l2-device.h +#include media/v4l2-ioctl.h + +#define ADV761X_DRIVER_NAME adv761x + +/* VERT_FILTER_LOCKED and DE_REGEN_FILTER_LOCKED flags */ +#define ADV761X_HDMI_F_LOCKED(v) (((v) 0xa0) == 0xa0) + +/* Maximum supported resolution */ +#define ADV761X_MAX_WIDTH1920 +#define ADV761X_MAX_HEIGHT 1080 + +static int debug; +module_param(debug, int, 0644); +MODULE_PARM_DESC(debug, debug level (0-2)); + +/* I2C map addresses */ +static u8 i2c_cec = 0x40; +module_param(i2c_cec, byte, 0644); +MODULE_PARM_DESC(i2c_cec, CEC map 7-bit I2C address (default: 0x40)); + +static u8 i2c_inf = 0x3e; +module_param(i2c_inf, byte, 0644); +MODULE_PARM_DESC(i2c_inf, InfoFrame map 7-bit I2C address (default: 0x3e)); + +static u8 i2c_dpll = 0x26; +module_param(i2c_dpll, byte, 0644); +MODULE_PARM_DESC(i2c_dpll, DPLL map 7-bit I2C address (default: 0x20)); + +static u8 i2c_rep = 0x32; +module_param(i2c_rep, byte, 0644); +MODULE_PARM_DESC(i2c_rep, Repeater map 7-bit I2C address (default: 0x32)); + +static u8 i2c_edid = 0x36; +module_param(i2c_edid, byte, 0644); +MODULE_PARM_DESC(i2c_edid, EDID map 7-bit I2C address (default: 0x36)); + +static u8 i2c_hdmi = 0x34; +module_param(i2c_hdmi, byte, 0644); +MODULE_PARM_DESC(i2c_hdmi, HDMI map 7-bit I2C address (default: 0x34)); + +static u8 i2c_cp = 0x22;
Re: [PATCH] media: i2c: adv7343: fix the DT binding properties
On Mon, Sep 23, 2013 at 01:50:51PM +0200, Laurent Pinchart wrote: Isn't regulator_get_optional() intended for devices that can have supplies unconnected in normal use ? The ADV7343 supplies are mandatory from a hardware point of view, so I think we should use regulator_get(). Otherwise the driver won't be able to tell the difference between a regulator that isn't present yet (for instance because the regulator device/driver hasn't been probed yet), which should result in deferred probing, and an always-on regulator that has been left out. Yes, everything you say here is correct. signature.asc Description: Digital signature
Re: [PATCH] Revert V4L/DVB: uvc: Enable USB autosuspend by default on uvcvideo
On Tue, Sep 24, 2013 at 4:34 AM, Laurent Pinchart laurent.pinch...@ideasonboard.com wrote: I've discussed this issue during LPC last week, and I still believe we should enable auto-suspend. The feature really saves power, without it my C910 Logitech webcam gets hot even when unused. If we disable auto-suspend by default and enable it from userspace only a handful of devices will get auto-suspended. Unless we can get distros to automatically test auto-suspend on unknown webcam models and report the results to update a central data base (which would grow much bigger than a quirks list in the driver in my opinion), disabling auto-suspend would be a serious regression. Setting defaults which knowingly cause problems is a horrible idea. Just because it works for you and your setup is no justification to force it upon everyone. This is certainly a feature that, if wanted, can be enabled by the user. I don't see any reasonable argument against letting the user enable it if he/she wants it. Ps. Sorry for the double-post if anyone already received this. The first posting wasn't using plain text and was therefore rejected so this corrects that. -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/3] media: rcar_vin: Add RGB888_1X24 input format support
On Tue, 24 Sep 2013, Valentine Barshak wrote: This adds V4L2_MBUS_FMT_RGB888_1X24 input format support which is used by the ADV7612 chip. Signed-off-by: Valentine Barshak valentine.bars...@cogentembedded.com --- drivers/media/platform/soc_camera/rcar_vin.c | 12 ++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/drivers/media/platform/soc_camera/rcar_vin.c b/drivers/media/platform/soc_camera/rcar_vin.c index cf81e02..0f04cff 100644 --- a/drivers/media/platform/soc_camera/rcar_vin.c +++ b/drivers/media/platform/soc_camera/rcar_vin.c @@ -68,6 +68,7 @@ #define VNMC_INF_YUV8_BT656 (0 16) #define VNMC_INF_YUV8_BT601 (1 16) #define VNMC_INF_YUV16 (5 16) +#define VNMC_INF_RGB888 (6 16) #define VNMC_VUP (1 10) #define VNMC_IM_ODD (0 3) #define VNMC_IM_ODD_EVEN (1 3) @@ -235,7 +236,7 @@ static int rcar_vin_setup(struct rcar_vin_priv *priv) struct soc_camera_device *icd = priv-ici.icd; struct rcar_vin_cam *cam = icd-host_priv; u32 vnmc, dmr, interrupts; - bool progressive = false, output_is_yuv = false; + bool progressive = false, output_is_yuv = false, input_is_yuv = false; switch (priv-field) { case V4L2_FIELD_TOP: @@ -269,11 +270,17 @@ static int rcar_vin_setup(struct rcar_vin_priv *priv) case V4L2_MBUS_FMT_YUYV8_1X16: /* BT.601/BT.1358 16bit YCbCr422 */ vnmc |= VNMC_INF_YUV16; + input_is_yuv = true; break; case V4L2_MBUS_FMT_YUYV8_2X8: /* BT.656 8bit YCbCr422 or BT.601 8bit YCbCr422 */ vnmc |= priv-pdata-flags RCAR_VIN_BT656 ? VNMC_INF_YUV8_BT656 : VNMC_INF_YUV8_BT601; + input_is_yuv = true; + break; + case V4L2_MBUS_FMT_RGB888_1X24: + vnmc |= VNMC_INF_RGB888; + break; default: break; } This is actually not related to this patch, but I just looked at the driver again - at the rcar_vin_setup() function, the /* output format */ switch (icd-current_fmt-host_fmt-fourcc) { selection. The default case of that switch statement returns an error, whereas the driver claims to support a pass-through mode. That looks like a ubg to me, but it should be fixed in a separate patch. Thanks Guennadi @@ -316,7 +323,7 @@ static int rcar_vin_setup(struct rcar_vin_priv *priv) vnmc |= VNMC_VUP; /* If input and output use the same colorspace, use bypass mode */ - if (output_is_yuv) + if (input_is_yuv == output_is_yuv) vnmc |= VNMC_BPS; /* progressive or interlaced mode */ @@ -1002,6 +1009,7 @@ static int rcar_vin_get_formats(struct soc_camera_device *icd, unsigned int idx, switch (code) { case V4L2_MBUS_FMT_YUYV8_1X16: case V4L2_MBUS_FMT_YUYV8_2X8: + case V4L2_MBUS_FMT_RGB888_1X24: if (cam-extra_fmt) break; -- 1.8.3.1 --- Guennadi Liakhovetski, Ph.D. Freelance Open-Source Software Developer http://www.open-technology.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 0/3] media: Add SH-Mobile RCAR-H2 Lager board support
Hi Valentine, Since patches 2 and 3 here are for soc-camera, I can offer to take all 3 via my tree after all comments to patch 1/3 are addressed and someone (Laurent?) has acked it. Alternatively I can ack the two patches and let all 3 go via another tree, or we can split this series too - no problem with me either way. Thanks Guennadi On Tue, 24 Sep 2013, Valentine Barshak wrote: The following patches add ADV7611/ADV7612 HDMI receiver I2C driver and add RCAR H2 SOC support along with ADV761x output format support to rcar_vin soc_camera driver. These changes are needed for SH-Mobile R8A7790 Lager board video input support. Valentine Barshak (3): media: i2c: Add ADV761X support media: rcar_vin: Add preliminary r8a7790 H2 support media: rcar_vin: Add RGB888_1X24 input format support drivers/media/i2c/Kconfig| 11 + drivers/media/i2c/Makefile |1 + drivers/media/i2c/adv761x.c | 1060 ++ drivers/media/platform/soc_camera/rcar_vin.c | 17 +- include/media/adv761x.h | 28 + 5 files changed, 1114 insertions(+), 3 deletions(-) create mode 100644 drivers/media/i2c/adv761x.c create mode 100644 include/media/adv761x.h -- 1.8.3.1 --- Guennadi Liakhovetski, Ph.D. Freelance Open-Source Software Developer http://www.open-technology.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 1/3] media: i2c: Add ADV761X support
Hi Hans On Tue, 24 Sep 2013, Hans Verkuil wrote: Shouldn't the interrupt_service_routine() op be implemented as well? Usually these drivers will generate interrupts if e.g. the format changes. Should it? AFAIU, .interrupt_service_routine() is a subde operation to be called by a bridge driver, when it gets an interrupt for the respective subdevice: interrupt_service_routine: Called by the bridge chip's interrupt service handler, when an interrupt status has be raised due to this subdev, typo ^^ so that this subdev can handle the details. It may schedule work to be performed later. It must not sleep. *Called from an IRQ context*. In this case the device does indeed have 2 interrupt output lines, but I don't think they would be connected to dedicated bridge inputs, rather to GPIOs, so, the driver can implement an ISR itself, when it decides to implement support for those interrupts. Thanks Guennadi --- Guennadi Liakhovetski, Ph.D. Freelance Open-Source Software Developer http://www.open-technology.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 v5 1/4] media: s5p-tv: Replace mxr_ macro by default dev_
On Tue, 2013-09-24 at 14:52 +0200, Tomasz Stanislawski wrote: On 09/23/2013 07:44 PM, Joe Perches wrote: On Mon, 2013-09-23 at 17:48 +0200, Bartlomiej Zolnierkiewicz wrote: On Monday, September 23, 2013 04:50:01 PM Tomasz Stanislawski wrote: May I ask what is the rationale for this patch? To reduce a few lines of code? This patch makes source code more generic-like and easier to follow (mxd_r* macros currently only obfuscate the code and make them harder to read for everybody, maybe besides the original driver author ;). Removal of few superfluous lines of code is just a bonus. I don't see any significant issue with this change. Using generic mechanisms is good. Hi Joe, Hi Tomasz Sorry for flaming Sorry, but I didn't feel any heat. Maybe I'm too far from the fire. I'll get closer. but please let me explain reasons of my opposition to this patch. I don't have an issue either way. I prefer using localized logging macros like mxd_level but I don't much care if actual maintainers want to use the generic style or a localized style. 4. I looked for other files where macro for dev_err is used. I tried following shell command on v3.12-rc2. git grep -A1 _err( | grep -A1 '#define' | grep -B1 dev_err then processing results using grep -v ^-- | cut -d: -f 1 | sort -u | wc You'll have to look farther then 1 line for dev_err There are uses like: #define foo_err(pointer, fmt, ...) \ do {\ if (something) \ dev_err(pointer-something, ...); \ } while (0) Currently, due to mentioned reason the patch is not a cleanup-up for me. And since I am still a maintainer of this god-forgotten driver I am going NACK this patch because it makes my work more difficult and because this patch provides only (if any) relative aesthetic gain. Good for you. Maintainer preference trumps global style consistency. cheers, Joe -- 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 1/3] media: i2c: Add ADV761X support
On 09/24/2013 06:19 PM, Guennadi Liakhovetski wrote: Hi Hans On Tue, 24 Sep 2013, Hans Verkuil wrote: Shouldn't the interrupt_service_routine() op be implemented as well? Usually these drivers will generate interrupts if e.g. the format changes. Should it? AFAIU, .interrupt_service_routine() is a subde operation to be called by a bridge driver, when it gets an interrupt for the respective subdevice: interrupt_service_routine: Called by the bridge chip's interrupt service handler, when an interrupt status has be raised due to this subdev, typo ^^ so that this subdev can handle the details. It may schedule work to be performed later. It must not sleep. *Called from an IRQ context*. Hmm, I have serious doubts whether the It must not sleep. *Called from an IRQ context*. part is correct. I have to check that. I think it is actually the opposite, especially since most i2c drivers will have to do i2c accesses which can always sleep. In this case the device does indeed have 2 interrupt output lines, but I don't think they would be connected to dedicated bridge inputs, rather to GPIOs, so, the driver can implement an ISR itself, when it decides to implement support for those interrupts. That's another option. But this driver implements neither option, which is unusual. Regards, Hans -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/3] media: i2c: Add ADV761X support
Hans Verkuil hverk...@xs4all.nl wrote: On 09/24/2013 06:19 PM, Guennadi Liakhovetski wrote: Hi Hans On Tue, 24 Sep 2013, Hans Verkuil wrote: Shouldn't the interrupt_service_routine() op be implemented as well? Usually these drivers will generate interrupts if e.g. the format changes. Should it? AFAIU, .interrupt_service_routine() is a subde operation to be called by a bridge driver, when it gets an interrupt for the respective subdevice: interrupt_service_routine: Called by the bridge chip's interrupt service handler, when an interrupt status has be raised due to this subdev, typo ^^ so that this subdev can handle the details. It may schedule work to be performed later. It must not sleep. *Called from an IRQ context*. Hmm, I have serious doubts whether the It must not sleep. *Called from an IRQ context*. part is correct. I have to check that. I think it is actually the opposite, especially since most i2c drivers will have to do i2c accesses which can always sleep. In this case the device does indeed have 2 interrupt output lines, but I don't think they would be connected to dedicated bridge inputs, rather to GPIOs, so, the driver can implement an ISR itself, when it decides to implement support for those interrupts. That's another option. But this driver implements neither option, which is unusual. 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 The comment of the subdev irq handler op is (mine and) wrong. If a bridge driver knows a subdev has direct register access and doesn't sleep, then the op can be called from an irq context. (Although RT folks might not like it.) If a bridge driver doesn't know about the subdev implementation, then to be safe, the brige driver should call from a workqueue, threaded irq handler, kthread, etc. 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
[PATCH] [media] marvell-ccic: simplify and fix clk handling (a bit)
The marvell-ccic does several things wrong or ineffectively in the clock handling and it's usage of the devm_* stuff - it assumes clk_get doesn't return NULL - it explicitly calls devm_clk_put instead just keeping the reference during it's lifetime and let the driver core call it - it calls kfree, gpio_free and free_irq for resources it requested using devm_kzalloc, devm_gpio_request and devm_request_irq respectively. - it mixes devm_ and unmanaged resources which probably results in a race condition during remove This patch fixes all but the last issue in this list. This patch doesn't introduce new reasons for not building, but there are already several bulid problems. Signed-off-by: Uwe Kleine-König u.kleine-koe...@pengutronix.de --- Cc: Libin Yang lby...@marvell.com --- drivers/media/platform/marvell-ccic/mmp-driver.c | 26 ++-- 1 file changed, 2 insertions(+), 24 deletions(-) diff --git a/drivers/media/platform/marvell-ccic/mmp-driver.c b/drivers/media/platform/marvell-ccic/mmp-driver.c index b5a19af..49ff52e 100644 --- a/drivers/media/platform/marvell-ccic/mmp-driver.c +++ b/drivers/media/platform/marvell-ccic/mmp-driver.c @@ -143,7 +143,6 @@ static int mmpcam_power_up(struct mcam_camera *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); } @@ -186,12 +185,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); } @@ -325,19 +318,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 +351,7 @@ static int mmpcam_probe(struct platform_device *pdev) if (cam == NULL) return -ENOMEM; cam-pdev = pdev; - cam-mipi_clk = NULL; + cam-mipi_clk = devm_clk_get(pdev-dev, mipi); INIT_LIST_HEAD(cam-devlist); mcam = cam-mcam; @@ -442,6 +422,7 @@ static int mmpcam_probe(struct platform_device *pdev) /* * Power the device up and hand it off to the core. */ + ret = mmpcam_power_up(mcam); if (ret) goto out_deinit_clk; @@ -470,7 +451,6 @@ out_unregister: out_power_down: mmpcam_power_down(mcam); out_deinit_clk: - mcam_deinit_clk(mcam); return ret; } @@ -487,10 +467,8 @@ static int mmpcam_remove(struct mmp_camera *cam) pdata = cam-pdev-dev.platform_data; gpio_free(pdata-sensor_reset_gpio); gpio_free(pdata-sensor_power_gpio); - mcam_deinit_clk(mcam); iounmap(cam-power_regs); iounmap(mcam-regs); - kfree(cam); return 0; } -- 1.8.4.rc3 -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2] [media] marvell-ccic: simplify and fix clk handling (a bit)
The marvell-ccic does several things wrong or ineffectively in the clock handling and it's usage of the devm_* stuff - it assumes clk_get doesn't return NULL - it explicitly calls devm_clk_put instead just keeping the reference during it's lifetime and let the driver core call it - it calls kfree, gpio_free and free_irq for resources it requested using devm_kzalloc, devm_gpio_request and devm_request_irq respectively. - it mixes devm_ and unmanaged resources which probably results in a race condition during remove This patch fixes all but the last issue in this list. This patch doesn't introduce new reasons for not building, but there are already several bulid problems. Signed-off-by: Uwe Kleine-König u.kleine-koe...@pengutronix.de --- Cc: Libin Yang lby...@marvell.com Changes since (implicit) v1: - really fix the third issue in the list. - drop whitespace noise hunk --- drivers/media/platform/marvell-ccic/mmp-driver.c | 29 ++-- 1 file changed, 2 insertions(+), 27 deletions(-) diff --git a/drivers/media/platform/marvell-ccic/mmp-driver.c b/drivers/media/platform/marvell-ccic/mmp-driver.c index b5a19af..ed16f81e 100644 --- a/drivers/media/platform/marvell-ccic/mmp-driver.c +++ b/drivers/media/platform/marvell-ccic/mmp-driver.c @@ -143,7 +143,6 @@ static int mmpcam_power_up(struct mcam_camera *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); } @@ -186,12 +185,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); } @@ -325,19 +318,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 +351,7 @@ static int mmpcam_probe(struct platform_device *pdev) if (cam == NULL) return -ENOMEM; cam-pdev = pdev; - cam-mipi_clk = NULL; + cam-mipi_clk = devm_clk_get(pdev-dev, mipi); INIT_LIST_HEAD(cam-devlist); mcam = cam-mcam; @@ -442,6 +422,7 @@ static int mmpcam_probe(struct platform_device *pdev) /* * Power the device up and hand it off to the core. */ + ret = mmpcam_power_up(mcam); if (ret) goto out_deinit_clk; @@ -470,7 +451,6 @@ out_unregister: out_power_down: mmpcam_power_down(mcam); out_deinit_clk: - mcam_deinit_clk(mcam); return ret; } @@ -481,16 +461,11 @@ static int mmpcam_remove(struct mmp_camera *cam) struct mmp_camera_platform_data *pdata; mmpcam_remove_device(cam); - free_irq(cam-irq, mcam); mccic_shutdown(mcam); mmpcam_power_down(mcam); pdata = cam-pdev-dev.platform_data; - gpio_free(pdata-sensor_reset_gpio); - gpio_free(pdata-sensor_power_gpio); - mcam_deinit_clk(mcam); iounmap(cam-power_regs); iounmap(mcam-regs); - kfree(cam); return 0; } -- 1.8.4.rc3 -- 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 v4] media: st-rc: Add ST remote control driver
On 09/24/2013 02:08 AM, Srinivas KANDAGATLA wrote: Thanks Stephen, On 23/09/13 21:40, Stephen Warren wrote: On 09/19/2013 02:59 AM, Srinivas KANDAGATLA wrote: This patch adds support to ST RC driver, which is basically a IR/UHF receiver and transmitter. This IP (IRB) is common across all the ST parts for settop box platforms. IRB is embedded in ST COMMS IP block. It supports both Rx Tx functionality. In this driver adds only Rx functionality via LIRC codec. diff --git a/Documentation/devicetree/bindings/media/st-rc.txt b/Documentation/devicetree/bindings/media/st-rc.txt + - rx-mode: can be infrared or uhf. rx-mode should be present iff the + rx pins are wired up. + - tx-mode: should be infrared. tx-mode should be present iff the tx + pins are wired up. Should those property names be prefixed with st,; I assume they're specific to this binding rather than something generic that applies to all IR controller bindings? If you expect them to be generic, it's fine. Officially these bindings are not specified in ePAPR specs Well, there are plenty of properties we now consider generic that aren't in ePAPR... but I see no reason for not having these properties as generic ones. Are you ok with that? I suppose that infrared-vs-uhf is a concept that's probably common enough across any similar HW device, so it may make sense for these properties to be generic. If we do intend them to be generic, I'd suggest they be defined in some generic binding document though; perhaps something like bindings/media/ir.txt or bindings/media/remote-control.txt? That way, a HW-specific binding isn't the only place where a supposedly generic property is defined. -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
cron job: media_tree daily build: ERRORS
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 Sep 25 04:00:48 CEST 2013 git branch: test git hash: ffee921033e64edf8579a3b21c7f15d1a6c3ef71 gcc version:i686-linux-gcc (GCC) 4.8.1 sparse version: 0.4.5-rc1 host hardware: x86_64 host os:3.10.1 linux-git-arm-at91: ERRORS linux-git-arm-davinci: ERRORS linux-git-arm-exynos: ERRORS linux-git-arm-mx: ERRORS linux-git-arm-omap: ERRORS linux-git-arm-omap1: ERRORS linux-git-arm-pxa: ERRORS linux-git-blackfin: ERRORS linux-git-i686: OK linux-git-m32r: OK linux-git-mips: ERRORS linux-git-powerpc64: OK linux-git-sh: ERRORS linux-git-x86_64: OK linux-2.6.31.14-i686: ERRORS linux-2.6.32.27-i686: ERRORS linux-2.6.33.7-i686: ERRORS linux-2.6.34.7-i686: ERRORS linux-2.6.35.9-i686: ERRORS linux-2.6.36.4-i686: ERRORS linux-2.6.37.6-i686: ERRORS linux-2.6.38.8-i686: ERRORS linux-2.6.39.4-i686: ERRORS linux-3.0.60-i686: ERRORS linux-3.10.1-i686: ERRORS linux-3.1.10-i686: ERRORS linux-3.11.1-i686: ERRORS linux-3.12-rc1-i686: OK linux-3.2.37-i686: ERRORS linux-3.3.8-i686: ERRORS linux-3.4.27-i686: ERRORS linux-3.5.7-i686: ERRORS linux-3.6.11-i686: ERRORS linux-3.7.4-i686: ERRORS linux-3.8-i686: ERRORS linux-3.9.2-i686: ERRORS linux-2.6.31.14-x86_64: ERRORS linux-2.6.32.27-x86_64: ERRORS linux-2.6.33.7-x86_64: ERRORS linux-2.6.34.7-x86_64: ERRORS linux-2.6.35.9-x86_64: ERRORS linux-2.6.36.4-x86_64: ERRORS linux-2.6.37.6-x86_64: ERRORS linux-2.6.38.8-x86_64: ERRORS linux-2.6.39.4-x86_64: ERRORS linux-3.0.60-x86_64: ERRORS linux-3.10.1-x86_64: ERRORS linux-3.1.10-x86_64: ERRORS linux-3.11.1-x86_64: ERRORS linux-3.12-rc1-x86_64: OK linux-3.2.37-x86_64: ERRORS linux-3.3.8-x86_64: ERRORS linux-3.4.27-x86_64: ERRORS linux-3.5.7-x86_64: ERRORS linux-3.6.11-x86_64: ERRORS linux-3.7.4-x86_64: ERRORS linux-3.8-x86_64: ERRORS linux-3.9.2-x86_64: ERRORS apps: WARNINGS spec-git: OK ABI WARNING: change for arm-at91 ABI WARNING: change for arm-davinci ABI WARNING: change for arm-exynos ABI WARNING: change for arm-mx ABI WARNING: change for arm-omap ABI WARNING: change for arm-omap1 ABI WARNING: change for arm-pxa ABI WARNING: change for blackfin ABI WARNING: change for i686 ABI WARNING: change for m32r ABI WARNING: change for mips ABI WARNING: change for powerpc64 ABI WARNING: change for sh ABI WARNING: change for x86_64 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: Hauppauge HVR-900 HD and HVR 930C-HD with si2165
On 17.08.2013 13:30, Ulf wrote: Hi, I know the topic Hauppauge HVR-900 HD and HVR 930C-HD with si2165 demodulator was already discussed http://permalink.gmane.org/gmane.linux.drivers.video-input-infrastructure/40982 and http://permalink.gmane.org/gmane.linux.drivers.video-input-infrastructure/46266. Just for me as a confirmation nobody plans to work on a driver for si2165. Is there any chance how to push the development? Ulf Hi! I also bought one of these to find out it is not supported. But my plan is to try to write a driver for this. I want to get DVB-C working, but I also have DVB-T and analog reception available. My current status is I got it working in windows in qemu and did a usb snoop. I also have a second system to test it in windows vista directly on the hardware. Current status is documented here. http://www.linuxtv.org/wiki/index.php/Hauppauge_WinTV-HVR-930C-HD Until now I only have a component list summarized from this list. * Conexant http://www.linuxtv.org/wiki/index.php/Conexant CX231xx http://www.linuxtv.org/wiki/index.php/Conexant_CX2310x * Silicon Labs http://www.linuxtv.org/wiki/index.php?title=Silicon_Labsaction=editredlink=1 si2165 http://www.linuxtv.org/wiki/index.php/Silicon_Labs_si2165 (Multi-Standard DVB-T and DVB-C Demodulator) * NXP TDA18271 http://www.linuxtv.org/wiki/index.php/NXP/Philips_TDA182xx (silicon tuner IC, most likely i2c-addr: 0x60) * eeprom (windows driver reads 1kb, i2c-addr: 0x50) Is this correct? Did anyone open his device and can show pictures? I now need to know which component is at which i2c address. Windows driver does upload file hcw10mlD.rom of 16kb to device 0x44. Regards Matthias -- 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