Re: [PATCH 11/42] ARM: shmobile: Register PFC platform device
Hi Simon, On Tuesday 27 November 2012 11:26:28 Simon Horman wrote: > On Mon, Nov 26, 2012 at 11:34:36AM +0100, Laurent Pinchart wrote: > > On Monday 26 November 2012 10:02:05 Simon Horman wrote: > > > On Wed, Nov 21, 2012 at 01:43:15PM +0100, Laurent Pinchart wrote: > > > > On Wednesday 21 November 2012 14:16:33 Simon Horman wrote: > > > > > On Wed, Nov 21, 2012 at 03:27:12AM +0100, Laurent Pinchart wrote: > > > > > > Add arch code to register the PFC platform device instead of > > > > > > calling the driver directly. Platform device registration in the > > > > > > sh-pfc driver will be removed. > > > > > > > > > > I'm not really sure that I understand the motivation for > > > > > moving platform device registration from the driver into > > > > > mach-shmobile. Could you explain this a little? > > > > > > > > Sure. > > > > > > > > The traditional device model associates a driver with a device. For > > > > historical reasons mach-shmobile doesn't define and register a > > > > platform device for PFC hardware but calls an initialization function > > > > directly in the PFC driver, passing it what is essentially platform > > > > data, including resources. > > > > > > > > The PFC driver needs a struct device to pass to the pinctrl subsystem. > > > > As no struct device corresponding to the hardware is created by > > > > mach-shmobile, the driver creates one, registers it and registers > > > > itself as a platform driver. The probe function is thus called > > > > synchronously, with a valid struct platform_device. > > > > > > > > This is a hack that can't support device tree based instantiation, as > > > > the platform device will be created when the platform is populated > > > > from the DT in that case. To support DT (and to remove the hack), I've > > > > moved platform device registration to mach-shmobile as it should be, > > > > like already done for all (or most, I haven't checked if there's no > > > > similar hacks in other drivers) the platform devices. This allows > > > > converting a board to DT by just adding the PFC device node in the DT > > > > and removing the platform device registration call in board code. > > > > > > > > I hope this made the intend of this part of the patch series clear. If > > > > not, just tell me and I'll try to provide more explanations. > > > > > > Thanks Laurent, > > > > > > as it happens I was doing some work on pinmux and DT in as part of > > > my kzm9g series, so what you describe above now makes a lot of sense to > > > me. > > > > > > For this and all the other shmobile patches in this series: > > > > > > Acked-by: Simon Horman > > > > Thank you. I'll post a v2 of the patch set with board patches split > > per-SoC as requested by Magnus to make backporting easier. As the > > shmobile will significantly change, could you send me your ack on v2 ? > > > > > BTW, my kzm9g work is not intended to conflict with your work in any way > > > and I apologise if it does. I was just trying to make something quickly > > > to allow kzm9g DT work to move a little further forward. I very much > > > welcome your work in this area and naturally the kzm9g will use it once > > > it is ready. > > > > No worries. I'll handle the conflict. Do you plan to push it for v3.8 or > > v3.9 ? > > Its too late for 3.8, so I was thinking about 3.9. > > I have rebased things on your v2 series and things seem to be working. Great :-) > So I'm now dependent on your pinmux work. OK, let's push the patches for v3.9 then. I'll wait for feedback on v2 and will submit a v3 with OF support for pinmuxing before the end of the week. -- Regards, Laurent Pinchart -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/5] backlight: Add GPIO-based backlight driver
Hi Jingoo,
On Tuesday 27 November 2012 01:10:36 Jingoo Han wrote:
> On Saturday, November 24, 2012 1:35 AM, Laurent Pinchart wrote
>
> >
> > Signed-off-by: Laurent Pinchart
> > ---
> >
> > drivers/video/backlight/Kconfig |7 ++
> > drivers/video/backlight/Makefile |1 +
> > drivers/video/backlight/gpio_backlight.c | 158 +
> > include/video/gpio_backlight.h | 21
> > 4 files changed, 187 insertions(+), 0 deletions(-)
> > create mode 100644 drivers/video/backlight/gpio_backlight.c
> > create mode 100644 include/video/gpio_backlight.h
>
>
> [...]
>
>
> > +static int __devinit gpio_backlight_probe(struct platform_device *pdev)
> > +{
> > + struct gpio_backlight_platform_data *pdata = pdev->dev.platform_data;
> > + struct backlight_properties props;
> > + struct backlight_device *bl;
> > + struct gpio_backlight *gbl;
> > + int ret;
> > +
> > + gbl = devm_kzalloc(&pdev->dev, sizeof(*gbl), GFP_KERNEL);
> > + if (gbl == NULL)
> > + return -ENOMEM;
> > +
> > + gbl->dev = &pdev->dev;
> > +
> > + if (!pdata) {
> > + dev_err(&pdev->dev, "failed to find platform data\n");
> > + return -ENODEV;
> > + }
> > +
> > + gbl->fbdev = pdata->fbdev;
> > + gbl->gpio = pdata->gpio;
> > + gbl->active = pdata->active_low ? 0 : 1;
> > +
> > + ret = devm_gpio_request_one(gbl->dev, gbl->gpio, GPIOF_DIR_OUT,
> > + pdata->name);
>
>
> Please use GPIOF_INIT flags if you want to turn off GPIO backlight.
> If gbl->active is inverted, GPIOF_INIT_HIGH can be used as below:
>
> ret = devm_gpio_request_one(gbl->dev, gbl->gpio,
> GPIOF_DIR_OUT | (gbl->active ?
> GPIOF_INIT_LOW : GPIOF_INIT_HIGH),
> pdata->name);
Good point, thank you. I'll fix that.
> > + if (ret < 0) {
> > + dev_err(&pdev->dev, "unable to request GPIO\n");
> > + return ret;
> > + }
> > +
> > + memset(&props, 0, sizeof(props));
> > + props.type = BACKLIGHT_RAW;
> > + props.max_brightness = 1;
> > + bl = backlight_device_register(dev_name(&pdev->dev), &pdev->dev, gbl,
> > + &gpio_backlight_ops, &props);
> > + if (IS_ERR(bl)) {
> > + dev_err(&pdev->dev, "failed to register backlight\n");
> > + return PTR_ERR(bl);
> > + }
> > +
> > + bl->props.brightness = pdata->def_value;
> > + backlight_update_status(bl);
> > +
> > + platform_set_drvdata(pdev, bl);
> > + return 0;
> > +}
--
Regards,
Laurent Pinchart
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2] UVC: use GFP_ATOMIC under spin lock.
Hi Cyril,
Thank you for the patch.
On Sunday 25 November 2012 02:58:19 Cyril Roelandt wrote:
> Found using the following semantic patch:
>
> @@
> @@
> spin_lock_irqsave(...);
> ... when != spin_unlock_irqrestore(...);
> * GFP_KERNEL
>
>
> Signed-off-by: Cyril Roelandt
Acked-by: Laurent Pinchart
and applied to my tree
> ---
> drivers/usb/gadget/uvc_video.c |3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/usb/gadget/uvc_video.c b/drivers/usb/gadget/uvc_video.c
> index b0e53a8..cd067a6 100644
> --- a/drivers/usb/gadget/uvc_video.c
> +++ b/drivers/usb/gadget/uvc_video.c
> @@ -309,7 +309,8 @@ uvc_video_pump(struct uvc_video *video)
> video->encode(req, video, buf);
>
> /* Queue the USB request */
> - if ((ret = usb_ep_queue(video->ep, req, GFP_KERNEL)) < 0) {
> + ret = usb_ep_queue(video->ep, req, GFP_ATOMIC);
> + if (ret < 0) {
> printk(KERN_INFO "Failed to queue request (%d)\n", ret);
> usb_ep_set_halt(video->ep);
> spin_unlock_irqrestore(&video->queue.irqlock, flags);
--
Regards,
Laurent Pinchart
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCHv9 1/3] Runtime Interpreted Power Sequences
well supported in the Linux kernel, I've implemented DBI support as part of the CDF RFC, and DSI support is missing. If you look at the DBI bus implementation the amount of code isn't overly large. > At least for Tegra, and I think the same holds for a wide variety of other > SoCs, dumb panels would be enough for a start. In the interest of getting a > working solution for those setups, maybe we can start small and add just > enough framework to register dumb panel drivers to along with code to wire > up a backlight to light up the display. Then we could possibly still make it > to have a proper solution to support the various LVDS panels for Tegra with > 3.9. > > I'm adding Laurent on Cc since he's probably busy working on a new > proposal for the panel/display framework. Maybe he can share his thought > on this. > > All of the above said, I'm willing to help out with the coding if that's > what is required to reach a solution that everybody can be happy with. That's much appreciated. If that means a couple additional hours of sleep per week for me I'll be extremely thankful :-) -- Regards, Laurent Pinchart signature.asc Description: This is a digitally signed message part.
Re: [PATCHv9 1/3] Runtime Interpreted Power Sequences
Hi Tomi, On Wednesday 21 November 2012 14:04:17 Tomi Valkeinen wrote: > On 2012-11-21 13:40, Thierry Reding wrote: [snip] > > One thing that's not very clear is how the backlight subsystem should be > > wired up with the display framework. I have a patch on top of the Tegra > > DRM driver which adds some ad-hoc display support using this power > > sequences series and the pwm-backlight. > > I think that's a separate issue: how to associate the lcd device and > backlight device together. I don't have a clear answer to this. > > There are many ways the backlight may be handled. In some cases the > panel and the backlight are truly independent, and you can use the other > without using the other (not very practical, though =). >From a control point of view that's always the case for DPI panels (as those panels have no control bus, the backlight control bus is by definition separate) and is the case for the two DBI panels I've seen (but I won't claim that's a significative number of panels). > But then with some LCDs the backlight may be controlled by sending commands > to the panel, and in this case the two may be quite linked. Changing the > backlight requires the panel driver to be up and running, and sometimes the > sending the backlight commands may need to be (say, DSI display, with > backlight commands going over the DSI bus). When you write "sending commands to the panel", do you mean on the same control bus that the panel use ? Or would that also include for instance an I2C backlight controller integrated inside a DSI panel module ? In the later case there might still be dependencies between the panel controller and the backlight controller (let's say for instance that the panel controller has a DSI-controller GPIO wired to the backlight controller reset signal), but in practice I don't know if that's ever the case. > So my feeling is that the panel driver should know about the related > backlight device. In the first case the panel driver would just call > enable/disable in the backlight device when the panel is turned on. That makes sense. Unless I'm mistaken a backlight is always associated with a panel (it would be called a light if there was no panel in front of it). We can thus expose backlight operations in the panel CDF (in-kernel) API. The panel driver would need a way to retrieve a pointer to the associated backlight device. > In the second case of the DSI panel... I'm not sure. I've implemented it > so that the panel driver creates the backlight device, and implements > the backlight callbacks. It then sends the DSI commands from those > callbacks. If we decide to make the panel expose backlight operations we could get rid of the backlight device in this case. > > From reading the proposal for the panel/display framework, it sounds > > like a lot more is planned than just enabling or disabling panels, but > > it also seems like a lot of code needs to be written to support things > > like DSI, DBI or other control busses. > > > > At least for Tegra, and I think the same holds for a wide variety of > > other SoCs, dumb panels would be enough for a start. In the interest of > > getting a working solution for those setups, maybe we can start small > > and add just enough framework to register dumb panel drivers to along > > with code to wire up a backlight to light up the display. Then we could > > possibly still make it to have a proper solution to support the various > > LVDS panels for Tegra with 3.9. > > Yes, we (Laurent and me) both agree that we should start simple. > > However, the common panel framework is not strictly needed for this. I'm > not sure of the current architecture for Tegra, but for OMAP we already > have panel drivers (omap specific ones, though). The panel drivers may > support multiple models, (for example, > drivers/video/omap2/displays/panel-generic-dpi.c). > > I don't see any problem with adding small Tegra specific panel drivers > for the time being, with the intention of converting to common panel > framework when that's available. I'm fine with that, but using the CDF would be even better :-) > Of course, the DT side is an issue. If you now create DT bindings for a > temporary model, and need to change it again later, you'll have some > headaches trying managing that without breaking the old bindings... This > is why I haven't pushed DT bindings for OMAP, as I know I have to change > them in the near future. -- Regards, Laurent Pinchart signature.asc Description: This is a digitally signed message part.
Re: [PATCHv9 1/3] Runtime Interpreted Power Sequences
rt to see common patterns. > > > > > > One thing that's not very clear is how the backlight subsystem should be > > > wired up with the display framework. I have a patch on top of the Tegra > > > DRM driver which adds some ad-hoc display support using this power > > > sequences series and the pwm-backlight. > > > > I think that's a separate issue: how to associate the lcd device and > > backlight device together. I don't have a clear answer to this. > > > > There are many ways the backlight may be handled. In some cases the > > panel and the backlight are truly independent, and you can use the other > > without using the other (not very practical, though =). > > At least for DT I think we can easily wire that up. I've actually posted > a patch recently that does so. I think in most cases it makes sense to > control them together, such as on DPMS changes, where you really want to > turn both the backlight and the LCD off, independent of how they are > tied together. > > > But then with some LCDs the backlight may be controlled by sending > > commands to the panel, and in this case the two may be quite linked. > > Changing the backlight requires the panel driver to be up and running, > > and sometimes the sending the backlight commands may need to be (say, > > DSI display, with backlight commands going over the DSI bus). > > > > So my feeling is that the panel driver should know about the related > > backlight device. In the first case the panel driver would just call > > enable/disable in the backlight device when the panel is turned on. > > Exactly. > > > In the second case of the DSI panel... I'm not sure. I've implemented it > > so that the panel driver creates the backlight device, and implements > > the backlight callbacks. It then sends the DSI commands from those > > callbacks. > > That certainly sounds like the right approach to me. > > > > From reading the proposal for the panel/display framework, it sounds > > > like a lot more is planned than just enabling or disabling panels, but > > > it also seems like a lot of code needs to be written to support things > > > like DSI, DBI or other control busses. > > > > > > At least for Tegra, and I think the same holds for a wide variety of > > > other SoCs, dumb panels would be enough for a start. In the interest of > > > getting a working solution for those setups, maybe we can start small > > > and add just enough framework to register dumb panel drivers to along > > > with code to wire up a backlight to light up the display. Then we could > > > possibly still make it to have a proper solution to support the various > > > LVDS panels for Tegra with 3.9. > > > > Yes, we (Laurent and me) both agree that we should start simple. > > > > However, the common panel framework is not strictly needed for this. I'm > > not sure of the current architecture for Tegra, but for OMAP we already > > have panel drivers (omap specific ones, though). The panel drivers may > > support multiple models, (for example, > > drivers/video/omap2/displays/panel-generic-dpi.c). > > > > I don't see any problem with adding small Tegra specific panel drivers > > for the time being, with the intention of converting to common panel > > framework when that's available. > > I can take a look at how such a driver could be implemented, but again, > I'm a bit reluctant to add something ad-hoc now when maybe we can have > it supported in a proper framework not too far away in the future. > > > Of course, the DT side is an issue. If you now create DT bindings for a > > temporary model, and need to change it again later, you'll have some > > headaches trying managing that without breaking the old bindings... This > > is why I haven't pushed DT bindings for OMAP, as I know I have to change > > them in the near future. > > We're already keeping back on this and none of the patches that define > the bindings have been merged yet. Although bindings have been known to > change every once in a while even for code that is already in mainline. -- Regards, Laurent Pinchart signature.asc Description: This is a digitally signed message part.
Re: [PATCHv9 1/3] Runtime Interpreted Power Sequences
Hi Tomi, On Tuesday 27 November 2012 17:19:05 Tomi Valkeinen wrote: > On 2012-11-27 17:08, Laurent Pinchart wrote: > > On Wednesday 21 November 2012 14:04:17 Tomi Valkeinen wrote: > >> On 2012-11-21 13:40, Thierry Reding wrote: > > [snip] > > > >>> One thing that's not very clear is how the backlight subsystem should be > >>> wired up with the display framework. I have a patch on top of the Tegra > >>> DRM driver which adds some ad-hoc display support using this power > >>> sequences series and the pwm-backlight. > >> > >> I think that's a separate issue: how to associate the lcd device and > >> backlight device together. I don't have a clear answer to this. > >> > >> There are many ways the backlight may be handled. In some cases the > >> panel and the backlight are truly independent, and you can use the other > >> without using the other (not very practical, though =). > > > > From a control point of view that's always the case for DPI panels (as > > those panels have no control bus, the backlight control bus is by > > definition separate) and is the case for the two DBI panels I've seen > > (but I won't claim that's a significative number of panels). > > They may have a control bus, I2C, SPI, etc. In some cases that can be > used to control the backlight. But yes, it's separate from the video bus. > > >> But then with some LCDs the backlight may be controlled by sending > >> commands to the panel, and in this case the two may be quite linked. > >> Changing the backlight requires the panel driver to be up and running, > >> and sometimes the sending the backlight commands may need to be (say, DSI > >> display, with backlight commands going over the DSI bus). > > > > When you write "sending commands to the panel", do you mean on the same > > control bus that the panel use ? Or would that also include for instance > > an I2C backlight controller integrated inside a DSI panel module ? In the > > later > > I mean the same control bus that is used to control the panel, be it shared > with video bus like DSI, or separate like I2C. > > > case there might still be dependencies between the panel controller and > > the backlight controller (let's say for instance that the panel controller > > has a DSI-controller GPIO wired to the backlight controller reset signal), > > but in practice I don't know if that's ever the case. > > > >> So my feeling is that the panel driver should know about the related > >> backlight device. In the first case the panel driver would just call > >> enable/disable in the backlight device when the panel is turned on. > > > > That makes sense. Unless I'm mistaken a backlight is always associated > > with a panel (it would be called a light if there was no panel in front > > of it). We can thus expose backlight operations in the panel CDF > > (in-kernel) API. The panel driver would need a way to retrieve a pointer > > to the associated backlight device. > > I agree. > > >> In the second case of the DSI panel... I'm not sure. I've implemented it > >> so that the panel driver creates the backlight device, and implements > >> the backlight callbacks. It then sends the DSI commands from those > >> callbacks. > > > > If we decide to make the panel expose backlight operations we could get > > rid of the backlight device in this case. > > Do you mean there would be a real backlight device only when there's a > totally independent backlight for the panel? I'm toying with that idea. I see two reasons for having backlight devices in the kernel right now. - Integration with the fbdev API to control the backlight automatically on fbdev blank/unblank/suspend/resume. That's something I want to get rid of, backlight should not depend on fbdev. If the backlight in-kernel API is exposed by the panel through CDF operations we won't need this anymore. - Exposing backlight control to userspace in sysfs. That's quite probably something we want to keep, if only for compatibility reasons. Backlight on/off should be controlled through the display APIs through DPMS control, but we have no way that I'm aware of to control the backlight brightness in the FBDEV and KMS APIs. It might make sense to add a backlight API to KMS for the future. One possibly crazy idea I had was to replace backlight devices as we know them with LED devices (a LED driver IC shouldn't be supported by different APIs depending on whether the LEDs it drives are used as a backlight or not), and have the panel hook up with the associated LED device to expose backlight operations through the CDF. Panels with a backlight controller tied to the panel controller (control through DSI for instance) would implement the backlight operations directly without instantiating a LED device. This still leaves the question of the userspace backlight brightness API open. -- Regards, Laurent Pinchart signature.asc Description: This is a digitally signed message part.
Re: [PATCH v3 9/9] davinci: vpfe: Add documentation and TODO
Hi Mauro,
Please see below.
On Wednesday 28 November 2012 09:22:13 Mauro Carvalho Chehab wrote:
> Hi Prabhakar,
>
> Em Wed, 28 Nov 2012 16:12:09 +0530
>
> Prabhakar Lad escreveu:
> > +Introduction
> > +
> > +
> > + This file documents the Texas Instruments Davinci Video processing Front
> > + End (VPFE) driver located under drivers/media/platform/davinci. The
> > + original driver exists for Davinci VPFE, which is now being changed to
> > + Media Controller Framework.
>
> Hmm... please correct me if I'm wrong, but are you wanting to replace an
> existing driver at drivers/media/platform/davinci, by another one at
> staging that has lots of known issues, as pointed at your TODO
>
> If so, please don't do that. Replacing a driver by some other one is
> generally a very bad idea, especially in this case, where the new driver
> has clearly several issues, the main one being to define its own proprietary
> and undocumented API:
>
> > +As of now since the interface will undergo few changes all the include
> > +files are present in staging itself, to build for dm365 follow below
> > +steps,
> > +
> > +- copy vpfe.h from drivers/staging/media/davinci_vpfe/ to
> > + include/media/davinci/ folder for building the uImage.
> > +- copy davinci_vpfe_user.h from drivers/staging/media/davinci_vpfe/ to
> > + include/uapi/linux/davinci_vpfe.h, and add a entry in Kbuild (required
> > + for building application).
> > +- copy dm365_ipipeif_user.h from drivers/staging/media/davinci_vpfe/ to
> > + include/uapi/linux/dm365_ipipeif.h and a entry in Kbuild (required
> > + for building application).
>
> Among other things, with those ugly and very likely mandatory API calls:
>
> >+/*
> >+ * Private IOCTL
> >+ * VIDIOC_VPFE_IPIPEIF_S_CONFIG: Set IPIEIF configuration
> >+ * VIDIOC_VPFE_IPIPEIF_G_CONFIG: Get IPIEIF configuration
> >+ */
> >+#define VIDIOC_VPFE_IPIPEIF_S_CONFIG \
> >+_IOWR('I', BASE_VIDIOC_PRIVATE + 1, struct ipipeif_params)
> >+#define VIDIOC_VPFE_IPIPEIF_G_CONFIG \
> >+_IOWR('I', BASE_VIDIOC_PRIVATE + 2, struct ipipeif_params)
> >+
> >+#endif
>
> I remember we rejected already drivers like that with obscure "S_CONFIG"
> private ioctl that were suspect to send a big initialization undocumented
> blob to the driver, as only the vendor's application would be able to use
> such driver.
That's correct, and that's why the driver is going to staging. From there it
will be incrementally fixed and then moved to drivers/media/, or dropped if
not maintained.
> So, instead, of submitting it to staging, you should be sending incremental
> patches for the existing driver, adding newer functionality there, and
> using the proper V4L2 API, with makes life easier for reviewers and
> application developers.
I agree that it would be the best thing to do, but I don't think it's going to
happen. We need to decide between two options.
- Push back now and insist in incremental patches for the existing driver, and
get nothing back as TI will very likely give up completely.
- Accept the driver in staging, get it fixed incrementally, and finally move
it to drivers/media/
There's a political side to this issue, we need to decide whether we want to
insist vendors getting everything right before any code reaches mainline, in
which case I believe we will lose some of them in the process, including major
vendors such as TI, or if we can make the mainline learning curve and
experience a bit more smooth by accepting such code in staging.
I would vote for the second option, with a very clear rule that getting the
driver in staging is only one step in the journey: if the development effort
stops there, the driver *will* be removed.
--
Regards,
Laurent Pinchart
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v3 0/9] Media Controller capture driver for DM365
On Wednesday 28 November 2012 16:12:00 Prabhakar Lad wrote: > From: Manjunath Hadli For staging, and provided that all parties involved understand that an API compatibility layer with the existing drivers/media/platform/davinci/ driver (called the "existing driver") will need to be provided when this media controller aware driver will move out of staging to drivers/media/ and replace the existing driver, Acked-by: Laurent Pinchart > Mauro/Greg, > The below series of patches have gone through good amount of reviews, and > agreed by Laurent, Hans and Sakari to be part of the staging tree. I am > combining the patchs with the pull request so we can get them into the 3.8 > kernel. Please pull these patches.If you want a seperate pull request, > please let me know. > > This patch set adds media controller based capture driver for > DM365. > > This driver bases its design on Laurent Pinchart's Media Controller Design > whose patches for Media Controller and subdev enhancements form the base. > The driver also takes copious elements taken from Laurent Pinchart and > others' OMAP ISP driver based on Media Controller. So thank you all the > people who are responsible for the Media Controller and the OMAP ISP driver. > > Also, the core functionality of the driver comes from the arago vpfe capture > driver of which the isif capture was based on V4L2, with other drivers like > ipipe, ipipeif and Resizer. > > Changes for v2: > 1: Migrated the driver for videobuf2 usage pointed Hans. > 2: Changed the design as pointed by Laurent, Exposed one more subdevs >ipipeif and split the resizer subdev into three subdevs. > 3: Rearrganed the patch sequence and changed the commit messages. > 4: Changed the file architecture as pointed by Laurent. > > Changes for v3: > 1: Rebased on staging. > 2: Seprated out patches which would go into staging. > > The following changes since commit c6c22955f80f2db9614b01fe5a3d1cfcd8b3d848: > > [media] dma-mapping: fix dma_common_get_sgtable() conditional compilation > (2012-11-27 09:42:31 -0200) > > are available in the git repository at: > git://linuxtv.org/mhadli/v4l-dvb-davinci_devices.git vpfe_driver_staging > > Manjunath Hadli (9): > davinci: vpfe: add v4l2 capture driver with media interface > davinci: vpfe: add v4l2 video driver support > davinci: vpfe: dm365: add IPIPEIF driver based on media framework > davinci: vpfe: dm365: add ISIF driver based on media framework > davinci: vpfe: dm365: add IPIPE support for media controller driver > davinci: vpfe: dm365: add IPIPE hardware layer support > davinci: vpfe: dm365: resizer driver based on media framework > davinci: vpfe: dm365: add build infrastructure for capture driver > davinci: vpfe: Add documentation and TODO > > drivers/staging/media/Kconfig |2 + > drivers/staging/media/Makefile |1 + > drivers/staging/media/davinci_vpfe/Kconfig |9 + > drivers/staging/media/davinci_vpfe/Makefile|3 + > drivers/staging/media/davinci_vpfe/TODO| 34 + > .../staging/media/davinci_vpfe/davinci-vpfe-mc.txt | 154 ++ > .../staging/media/davinci_vpfe/davinci_vpfe_user.h | 1290 > drivers/staging/media/davinci_vpfe/dm365_ipipe.c | 1863 + > drivers/staging/media/davinci_vpfe/dm365_ipipe.h | 179 ++ > .../staging/media/davinci_vpfe/dm365_ipipe_hw.c| 1048 ++ > .../staging/media/davinci_vpfe/dm365_ipipe_hw.h| 559 ++ > drivers/staging/media/davinci_vpfe/dm365_ipipeif.c | 1071 ++ > drivers/staging/media/davinci_vpfe/dm365_ipipeif.h | 233 +++ > .../media/davinci_vpfe/dm365_ipipeif_user.h| 93 + > drivers/staging/media/davinci_vpfe/dm365_isif.c| 2104 + > drivers/staging/media/davinci_vpfe/dm365_isif.h| 203 ++ > .../staging/media/davinci_vpfe/dm365_isif_regs.h | 294 +++ > drivers/staging/media/davinci_vpfe/dm365_resizer.c | 1999 + > drivers/staging/media/davinci_vpfe/dm365_resizer.h | 244 +++ > drivers/staging/media/davinci_vpfe/vpfe.h | 86 + > .../staging/media/davinci_vpfe/vpfe_mc_capture.c | 740 +++ > .../staging/media/davinci_vpfe/vpfe_mc_capture.h | 97 + > drivers/staging/media/davinci_vpfe/vpfe_video.c| 1620 +++ > drivers/staging/media/davinci_vpfe/vpfe_video.h| 155 ++ > 24 files changed, 14081 insertions(+), 0 deletions(-) > create mode 100644 drivers/staging/media/davinci_vpfe/Kconfig > create mode 100644 drivers/staging/media/davinci_vpfe/Makefile > create mode 100644 drivers/staging/media/davinci_vpfe/TODO > create mode 100644 drivers/staging/media/davinc
Re: [PATCH 00/03] gpio: Renesas R-Car GPIO driver update
Hi Magnus, On Tuesday 19 March 2013 12:36:52 Magnus Damm wrote: > On Thu, Mar 14, 2013 at 10:13 PM, Laurent Pinchart wrote: > > On Thursday 14 March 2013 13:23:46 Magnus Damm wrote: > >> On Wed, Mar 13, 2013 at 9:58 PM, Laurent Pinchart wrote: > >> > On Wednesday 13 March 2013 20:32:03 Magnus Damm wrote: > >> >> gpio: Renesas R-Car GPIO driver update > >> >> > >> >> [PATCH 01/03] gpio: Renesas R-Car GPIO driver V2 > >> >> [PATCH 02/03] gpio: rcar: Use IRQCHIP_SET_TYPE_MASKED > >> >> [PATCH 03/03] gpio: rcar: Make use of devm functions > >> >> > >> >> This series updates the R-Car GPIO driver with various > >> >> changes kindly suggested by Laurent Pinchart. > >> >> > >> >> Patch [01/03] is a drop in replacement for V1 of the R-Car > >> >> GPIO driver. The flag in patch [02/03] is kept out of [01/03] > >> >> to avoid changing the behaviour of the driver between V1 and V2. > >> >> Also, devm support in [03/03] is kept out of [01/03] to make > >> >> sure back porting goes as smooth as possible. > >> > > >> > As I mentioned in a previous e-mail, all the devm_* functions used in > >> > 03/03 have been available since 2.6.30. Do you really need to port that > >> > driver to older kernels ? > >> > >> Well, it was earlier suggested to me that not using devm to begin with > >> is a safe way forward for back porting. Also, as the individual patch > >> shows, we save about 10 lines of code but add many more complex > >> dependencies. > >> > >> Simon, do you have any recommendation how for us regarding devm and > >> back porting? > >> > >> > Regarding 02/03, do you plan to squash it with 01/03 for the mainline > >> > submission ? > >> > >> Not unless someone puts a gun to my head. =) Of course, if a single > >> patch is really required then I will follow that, but I can't really > >> see why when we have a nice versioning control system that can help > >> our development in various ways. > >> > >> What is the reason behind you wanting to merge these patches? > >> > >> From my point of view, if any incremental patch was a bug fix then i > >> would of course request to fold it in, but in this case these are > >> feature patches that would be beneficial to keep as individual > >> commits. Keeping them separate allows us to bisect and also makes > >> partial back porting easier if needed. > > > > When submitting new drivers I usually try not to make the development > > history visible to mainline. It brings little additional value (beside > > possibly making backporting a bit easier, but in the devm_* case that > > shouldn't be a problem, unless Simon thinks otherwise) but adds review > > complexity, as reviewers need to validate the intermediate versions as > > well. More patches also mean more potential bisection breakages. > > Huh, it seems that my point of view is the total opposite. I see that > using incremental patches to show new development would make review > _easier_. Perhaps that's not the case for most people. > > Regarding bisection, having features broken out actually allows us to > bisect compared to a single commit. I see that as a feature. Of course > the developer needs to make sure that the incremental changes do not > cause any breakages, but if the developer can't do that then there are > other more serious issues that need to be fixed IMO. > > > In this case (assuming there would be no backporting issue) there's no > > need for mainline to see that we started with a version that didn't use > > devm_* and then modified the code. I see no added value in that. > > So say that you write a driver and add say 8 features on top of that > over time, wouldn't it make sense to share that information with other > people? That way any party can bisect and locate issues that may come > up. I find that useful regardless if the code is back ported or not. Long answer short: it depends :-) If you add features incrementally, submitting them as individual patches totally make sense. However, if your patches aim at simplifying the code instead of adding features (such as devm_* usage), individual patches make it more complex in my opinion. Reviewers will need to look at the intermediate, more complex code, to make sure that no bisection breakage points will be introduced. Squashing the patches together then make review easier (as there's less code to review) and don't really hurt bisection, as the simplification patches usually remove code and thus bugs. > Anyway, with this particular driver it doesn't really matter since the > complexity is very low. And now Simon has gotten back to us with updated > information about back porting. > > I will pull it all together into a V3. Thank you :-) -- Regards, Laurent Pinchart -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH RFC v2] media: tvp514x: add OF support
Hi Prabhakar, On Sunday 03 February 2013 15:43:49 Prabhakar Lad wrote: > On Sat, Feb 2, 2013 at 4:07 AM, Sylwester Nawrocki wrote: > > On 01/29/2013 02:07 PM, Prabhakar Lad wrote: > > [...] > > > >> diff --git a/Documentation/devicetree/bindings/media/i2c/tvp514x.txt > >> b/Documentation/devicetree/bindings/media/i2c/tvp514x.txt > >> new file mode 100644 > >> index 000..55d3ffd > >> --- /dev/null > >> +++ b/Documentation/devicetree/bindings/media/i2c/tvp514x.txt > >> @@ -0,0 +1,38 @@ > >> +* Texas Instruments TVP514x video decoder > >> + > >> +The TVP5146/TVP5146m2/TVP5147/TVP5147m1 device is high quality, > >> +single-chip digital video decoder that digitizes and decodes all popular > >> +baseband analog video formats into digital video component. The tvp514x > >> +decoder supports analog-to-digital (A/D) conversion of component RGB and > >> +YPbPr signals as well as A/D conversion and decoding of NTSC, PAL and > >> +SECAM composite and S-video into component YCbCr. > >> + > >> +Required Properties : > >> +- compatible: Must be "ti,tvp514x-decoder" > > > > There are no significant differences among TVP514* devices as listed > > above, you would like to handle above ? > > > > I'm just wondering if you don't need, for instance, two separate > > compatible properties, e.g. "ti,tvp5146-decoder" and "ti,tvp5147-decoder" > > ? > > There are few differences in init/power sequence tough, I would still > like to have single compatible property "ti,tvp514x-decoder", If you feel we > need separate property I will change it please let me know on this. If there's any difference between the chips that need to be handled in the driver, using two compatible properties is a good practice. Your driver will then be able to easily differentiate between the two chips, and there's no real drawback in doing so. -- Regards, Laurent Pinchart -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2] ADP1653 board code for Nokia RX-51
Hi Sakari, On Thursday 04 April 2013 01:22:28 Sakari Ailus wrote: > On Tue, Mar 26, 2013 at 12:07:01AM +0100, Laurent Pinchart wrote: > > On Sunday 24 March 2013 23:46:01 Sakari Ailus wrote: > > > Pali Rohár wrote: > > > > On Thursday 07 March 2013 23:18:27 Sakari Ailus wrote: > > > >> On Wed, Mar 06, 2013 at 10:44:41PM +0100, Sebastian Reichel wrote: > > > >>> On Wed, Mar 06, 2013 at 09:20:16PM +0100, Pali Rohár wrote: > > > >>>> On Wednesday 06 March 2013 21:12:06 Pali Rohár wrote: > > > >>>>> On Sunday 17 February 2013 20:03:03 Aaro Koskinen wrote: > > > >>>>>> On Sun, Feb 17, 2013 at 04:16:49PM +0100, Pali Rohár wrote: > > > >>>>>>> +/* > > > >>>>>>> + * arch/arm/mach-omap2/board-rx51-camera.c > > > >>>>>>> + * > > > >>>>>>> + * Copyright (C) 2008 Nokia Corporation > > > >>>>>>> + * > > > >>>>>>> + * Contact: Sakari Ailus > > > >>>>>>> + * Tuukka Toivonen > > > >>>>>> > > > >>>>>> You should put these people to CC... Just to see if the addresses > > > >>>>>> are still valid (which I doubt). > > > >>>>> > > > >>>>> Ok, trying :-) > > > >>>> > > > >>>> I got "Delivery Status Notification (Failure)" for both addresses. > > > >> > > > >> This is expected. > > > >> > > > >>> Sakari Ailus hosts some code on github [0], which has the following > > > >>> email address: > > > >>> sakari.ailus+gitori...@retiisi.org.uk > > > >>> > > > >>> I added it to this mail's CC. > > > >>> > > > >>> [0] https://gitorious.org/~sailus > > > >> > > > >> Nice to hear people are interested in this. ;-) > > > >> > > > >> The primary reason I haven't tried submitting this to mainline is > > > >> that ARM board code has a bad reputation these days. The N900 does > > > >> not have yet support for device tree (AFAIK), which also would > > > >> require a few bits and pieces on the flash driver to work. > > > >> > > > >> Also the sensor and lens drivers would need at least some work before > > > >> being ready for submission to mainline for camera to be usable. > > > >> Unfortunately I haven't had recently time to work on this. N9(50) > > > >> support has higher priority for myself. That, too, is pending the DT > > > >> support for the device. > > > >> > > > >> There's indeed more up-to-date code in my repository. Even if it's > > > >> not too close to mainline anymore it should be a better starting > > > >> point than the old kernel from MeeGo. > > > >> > > > >> https://gitorious.org/omap3camera/pages/Home> > > > > > > > > Hi, > > > > > > > > this board code is same in your git repository and on meego obs. > > > > > > > > Patch only adding support for adp1653 driver which is already in > > > > upstream kernel. > > > > > > > > Are there any other problems with this patch to go for upstream? > > > > > > A few more things comes to mind. We depend a little bit on actual board > > > code; it's not only static data. That's the configuration of the > > > external clock from the ISP to the sensor. That should move to the > > > common clock framework so that the ISP registers the clock and the > > > sensor driver can then use it. AFAIR Laurent has done some work on that. > > > > Yes. I hope to get the patches in v3.10. > > Cool! :) The patches have been posted to the linux-media mailing list. If the dependencies make it to v3.10 the OMAP3 ISP patches should get there too. > > > The peculiar detail of the rx51 is that there's a switch on the camera > > > CCP2 bus that selects either sensor (primary or secondary). Both sensors > > > are connected to the same receiver. That isn't properly modelled > > > currently at all, so that's why we have rx51_camera_set_xshutdown(). I > > > guess it'd still work if you only power (i.e. open) either of the > > > devices at a time, though. > > > > Have you thought about how we could model that ? > > Well, the two dependent gpios could be modelled as two independent ones ( > for sensor drivers), but setting the state of those gpios could fail, > gpio_set_value() still returns void. This isn't pretty perhaps but as a > result the initialisation of the secondary sensor to be powered up at the > same time will fail since it's in reset: the xshutdown of both sensors is > controlled by the same gpio as is the mux (AFAIR). > > So one N900 camera specific gpio driver would be needed. It'd be a very > simple driver. What do you think? I think I'll need to see how the GPIOs are wired up on the board. -- Regards, Laurent Pinchart -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH RFC] media: tvp514x: add OF support
Hi Prabhakar,
Thank you for the patch.
Sylwester and Guennadi have posted a DT bindings proposal for V4L2 devices.
Shouldn't you base this patch on those bindings ?
On Thursday 24 January 2013 14:42:20 Prabhakar Lad wrote:
> From: Lad, Prabhakar
>
> add OF support for the tvp514x driver.
>
> Signed-off-by: Lad, Prabhakar
> Cc: Hans Verkuil
> Cc: Laurent Pinchart
> Cc: Mauro Carvalho Chehab
> Cc: Guennadi Liakhovetski
> ---
> This patch is on top of following patches:
> 1: https://patchwork.kernel.org/patch/1930941/
> 2: http://patchwork.linuxtv.org/patch/16193/
> 3: https://patchwork.kernel.org/patch/1944901/
>
> .../devicetree/bindings/media/i2c/tvp514x.txt | 30 ++
> drivers/media/i2c/tvp514x.c| 60 +++--
> 2 files changed, 85 insertions(+), 5 deletions(-)
> create mode 100644 Documentation/devicetree/bindings/media/i2c/tvp514x.txt
>
> diff --git a/Documentation/devicetree/bindings/media/i2c/tvp514x.txt
> b/Documentation/devicetree/bindings/media/i2c/tvp514x.txt new file mode
> 100644
> index 000..3cce323
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/media/i2c/tvp514x.txt
> @@ -0,0 +1,30 @@
> +* Texas Instruments TVP514x video decoder
> +
> +The TVP5146/TVP5146m2/TVP5147/TVP5147m1 device is high quality, single-chip
> +digital video decoder that digitizes and decodes all popular baseband
> analog +video formats into digital video component. The tvp514x decoder
> supports analog- +to-digital (A/D) conversion of component RGB and YPbPr
> signals as well as A/D +conversion and decoding of NTSC, PAL and SECAM
> composite and S-video into +component YCbCr.
> +
> +Required Properties :
> +- compatible: Must be "ti,tvp514x-decoder"
> +- hsync-active: HSYNC Polarity configuration for current interface.
> +- vsync-active: VSYNC Polarity configuration for current interface.
> +- data-active: Clock polarity of the current interface.
> +
> +Example:
> +
> +i2c0@1c22000 {
> + ...
> + ...
> +
> + tvp514x@5c {
> + compatible = "ti,tvp514x-decoder";
> + reg = <0x5c>;
> + hsync-active = <1>; /* Active low (Defaults to 0) */
> + hsync-active = <1>; /* Active low (Defaults to 0) */
> + data-active = <0>; /* Active low (Defaults to 0) */
> + };
> + ...
> +};
> diff --git a/drivers/media/i2c/tvp514x.c b/drivers/media/i2c/tvp514x.c
> index a4f0a70..0e2b15c 100644
> --- a/drivers/media/i2c/tvp514x.c
> +++ b/drivers/media/i2c/tvp514x.c
> @@ -12,6 +12,7 @@
> * Hardik Shah
> * Manjunath Hadli
> * Karicheri Muralidharan
> + * Prabhakar Lad
> *
> * This package is free software; you can redistribute it and/or modify
> * it under the terms of the GNU General Public License version 2 as
> @@ -930,6 +931,50 @@ static struct tvp514x_decoder tvp514x_dev = {
>
> };
>
> +#if defined(CONFIG_OF)
> +static const struct of_device_id tvp514x_of_match[] = {
> + {.compatible = "ti,tvp514x-decoder", },
> + {},
> +}
> +MODULE_DEVICE_TABLE(of, tvp514x_of_match);
> +
> +static struct tvp514x_platform_data
> + *tvp514x_get_pdata(struct i2c_client *client)
> +{
> + if (!client->dev.platform_data && client->dev.of_node) {
> + struct tvp514x_platform_data *pdata;
> + u32 prop;
> +
> + pdata = devm_kzalloc(&client->dev,
> + sizeof(struct tvp514x_platform_data),
> + GFP_KERNEL);
> + client->dev.platform_data = pdata;
> + if (!pdata)
> + return NULL;
> + if (!of_property_read_u32(client->dev.of_node,
> + "data-active", &prop))
> + pdata->clk_polarity = prop;
> + if (!of_property_read_u32(client->dev.of_node,
> + "hsync-active", &prop))
> + pdata->hs_polarity = prop;
> + if (!of_property_read_u32(client->dev.of_node,
> + "vsync-active", &prop))
> + pdata->vs_polarity = prop;
> +
> + }
> +
> + return client->dev.platform_data;
> +}
> +#else
> +#define tvp514x_of_match NULL
> +
> +static struct tvp514x_platform_data
> + *tvp514x_get_pdata(struct i2c_client *client)
> +{
> + return client->dev.platform_data;
> +}
> +#endif
> +
> /**
> * tvp514x_probe() - decoder driver i2c probe handler
> * @client: i2c drive
Re: [PATCH] gpio: Renesas R-Car GPIO driver
Hi Magnus,
On Tuesday 12 March 2013 14:27:25 Magnus Damm wrote:
> On Sun, Mar 10, 2013 at 3:16 AM, Laurent Pinchart wrote:
> > Hi Magnus,
> >
> > Thank you for the patch.
>
> Thanks for your review!
You're welcome.
> > On Tuesday 05 March 2013 09:32:19 Magnus Damm wrote:
> >> From: Magnus Damm
> >>
> >> This patch implements a GPIO driver for the R-Car series
> >> of SoCs from Renesas. This driver is designed to be reusable
> >> between multiple SoCs that share the same basic building block,
> >> but so far it has only been used on R-Car H1 (r8a7779).
> >>
> >> Each driver instance handles 32 GPIOs with individually
> >> maskable IRQs. The driver operates on a single I/O memory
> >> range and the 32 GPIOs are hooked up a single interrupt.
> >>
> >> In the case of R-Car H1 either external IRQ pins or GPIOs
> >> with interrupts can be used for on-board interupts. For
> >> external IRQs 4 pins are supported, and in the case of GPIO
> >> there are 202 GPIOS as 202 interrupts hooked up via 6 driver
> >> instances and to the GIC and the Cortex-A9 Quad.
> >>
> >> At this point this driver is interfacing as a regular
> >> platform device driver. In the future DT support will be
> >> submitted as an incremental feature patch.
> >>
> >> Signed-off-by: Magnus Damm
> >> ---
> >>
> >> drivers/gpio/Kconfig|6
> >> drivers/gpio/Makefile |1
> >> drivers/gpio/gpio-rcar.c| 406 +
> >> include/linux/platform_data/gpio-rcar.h | 29 ++
> >> 4 files changed, 442 insertions(+)
> >>
> >> --- /dev/null
> >> +++ work/drivers/gpio/gpio-rcar.c 2013-03-05 09:13:23.0 +0900
> >> @@ -0,0 +1,406 @@
[snip]
> >> +static void gpio_rcar_config_interrupt_input_mode(struct gpio_rcar_priv
> >> *p, + unsigned int hwirq,
> >> + bool
> >> active_high_rising_edge, +
> >> bool level_trigger) +{
> >> + unsigned long bit = BIT(hwirq);
> >> + unsigned long flags, tmp;
> >> +
> >> + /* follow steps in the GPIO documentation for
> >> + * "Setting Edge-Sensitive Interrupt Input Mode" and
> >> + * "Setting Level-Sensitive Interrupt Input Mode"
> >> + */
> >> +
> >> + spin_lock_irqsave(&p->lock, flags);
> >> +
> >> + /* Configure postive or negative logic in POSNEG */
> >> + tmp = gpio_rcar_read(p, POSNEG);
> >> + if (active_high_rising_edge)
> >> + tmp &= ~bit;
> >> + else
> >> + tmp |= bit;
> >> + gpio_rcar_write(p, POSNEG, tmp);
> >> +
> >> + /* Configure edge or level trigger in EDGLEVEL */
> >> + tmp = gpio_rcar_read(p, EDGLEVEL);
> >> + if (level_trigger)
> >> + tmp &= ~bit;
> >> + else
> >> + tmp |= bit;
> >> + gpio_rcar_write(p, EDGLEVEL, tmp);
> >> +
> >> + /* Select "Interrupt Input Mode" in IOINTSEL */
> >> + tmp = gpio_rcar_read(p, IOINTSEL);
> >> + tmp |= bit;
> >> + gpio_rcar_write(p, IOINTSEL, tmp);
> >> +
> >> + /* Write INTCLR in case of edge trigger */
> >> + if (!level_trigger)
> >
> > Maybe you could do so unconditionally.
>
> I could, but I wouldn't follow the recommended sequence in the data sheet
> then.
> >> + gpio_rcar_write(p, INTCLR, bit);
> >
> > I suppose the idea here it to avoid spurious interrupts when switching to
> > edge-triggered interrupt mode. If the interrupt was already enabled
> > wouldn't it still be registered before you clear the flag ?
>
> Maybe. Perhaps I shall use IRQCHIP_SET_TYPE_MASKED here?
That's a good idea.
> >> +
> >> + spin_unlock_irqrestore(&p->lock, flags);
> >> +}
[snip]
> >> +static int gpio_rcar_get(struct gpio_chip *chip, unsigned offset)
> >> +{
> >> + return (int)(gpio_rcar_read(gpio_to_priv(chip), INDT) &
> >> BIT(offset));
> >> +}
> >
> > Isn't the get operation supposed to return 0 or 1 ? In that case
> >
> > return !!(gpio_rcar_read(gpio_to_priv(chip), INDT) &am
[PATCH] pinctrl: Print the correct information in debugfs pinconf-state file
A bad copy&paste resulted in the debugfs pinconf-state file printing the
pin name instead of the state name. Fix it.
Signed-off-by: Laurent Pinchart
---
drivers/pinctrl/pinconf.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/pinctrl/pinconf.c b/drivers/pinctrl/pinconf.c
index 8aefd28..dae927f 100644
--- a/drivers/pinctrl/pinconf.c
+++ b/drivers/pinctrl/pinconf.c
@@ -622,7 +622,7 @@ static const struct file_operations
pinconf_dbg_pinname_fops = {
static int pinconf_dbg_state_print(struct seq_file *s, void *d)
{
if (strlen(dbg_state_name))
- seq_printf(s, "%s\n", dbg_pinname);
+ seq_printf(s, "%s\n", dbg_state_name);
else
seq_printf(s, "No pin state set\n");
return 0;
--
Regards,
Laurent Pinchart
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 00/03] gpio: Renesas R-Car GPIO driver update
Hi Magnus, Thanks for the patches. On Wednesday 13 March 2013 20:32:03 Magnus Damm wrote: > gpio: Renesas R-Car GPIO driver update > > [PATCH 01/03] gpio: Renesas R-Car GPIO driver V2 > [PATCH 02/03] gpio: rcar: Use IRQCHIP_SET_TYPE_MASKED > [PATCH 03/03] gpio: rcar: Make use of devm functions > > This series updates the R-Car GPIO driver with various > changes kindly suggested by Laurent Pinchart. > > Patch [01/03] is a drop in replacement for V1 of the R-Car > GPIO driver. The flag in patch [02/03] is kept out of [01/03] > to avoid changing the behaviour of the driver between V1 and V2. > Also, devm support in [03/03] is kept out of [01/03] to make > sure back porting goes as smooth as possible. As I mentioned in a previous e-mail, all the devm_* functions used in 03/03 have been available since 2.6.30. Do you really need to port that driver to older kernels ? Regarding 02/03, do you plan to squash it with 01/03 for the mainline submission ? -- Regards, Laurent Pinchart -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 01/03] gpio: Renesas R-Car GPIO driver V2
Hi Magnus,
Thanks for the patch.
I've reviewed the result of squashing the 3 patches together, I just have one
comment.
On Wednesday 13 March 2013 20:32:13 Magnus Damm wrote:
> From: Magnus Damm
>
> This patch is V2 of a GPIO driver for the R-Car series of
> SoCs from Renesas. This driver is designed to be reusable
> between multiple SoCs that share the same basic building block,
> but so far it has only been used on R-Car H1 (r8a7779).
>
> Each driver instance handles 32 GPIOs with individually
> maskable IRQs. The driver operates on a single I/O memory
> range and the 32 GPIOs are hooked up a single interrupt.
>
> In the case of R-Car H1 either external IRQ pins or GPIOs
> with interrupts can be used for on-board interupts. For
> external IRQs 4 pins are supported, and in the case of GPIO
> there are 202 GPIOS as 202 interrupts hooked up via 6 driver
> instances and to the GIC and the Cortex-A9 Quad.
>
> At this point this driver is interfacing as a regular
> platform device driver. In the future DT support will be
> submitted as an incremental feature patch.
>
> Signed-off-by: Magnus Damm
> ---
>
> Changes since V1:
> - Update based on most suggestions from review by Laurent, thanks!
>
> Please note that in V2 the driver is reworked to reduce the number of
> lines and includes minor changes related to headers, printouts and
> data types. In V2 the register access order is kept the same as in V1,
> and to make that happen IRQCHIP_SET_TYPE_MASKED is omitted. The interface
> to the Linux kernel is unchanged except the use of dev_dbg() instead of
> pr_debug(). So from a hardware point of view V2 is a drop in replacement
> for V1 and there should be no additional dependencies since devm is
> kept as a separate patch as well. All this to make back porting easier.
>
> drivers/gpio/Kconfig|6
> drivers/gpio/Makefile |1
> drivers/gpio/gpio-rcar.c| 383
> include/linux/platform_data/gpio-rcar.h | 25 ++
> 4 files changed, 415 insertions(+)
[snip]
> --- /dev/null
> +++ work/drivers/gpio/gpio-rcar.c 2013-03-13 19:41:35.0 +0900
> @@ -0,0 +1,383 @@
> +/*
> + * Renesas R-Car GPIO Support
> + *
> + * Copyright (C) 2013 Magnus Damm
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License
> + *
> + * 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.
> + */
[snip]
> +static int gpio_rcar_irq_domain_map(struct irq_domain *h, unsigned int
> virq,
> + irq_hw_number_t hw)
> +{
> + struct gpio_rcar_priv *p = h->host_data;
> +
> + dev_dbg(&p->pdev->dev, "map hw irq = %d, virq = %d\n", (int)hw, virq);
> +
> + irq_set_chip_data(virq, h->host_data);
> + irq_set_chip_and_handler(virq, &p->irq_chip, handle_level_irq);
> + set_irq_flags(virq, IRQF_VALID); /* kill me now */
What is that comment about ?
> + return 0;
> +}
--
Regards,
Laurent Pinchart
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 00/03] gpio: Renesas R-Car GPIO driver update
Hi Magnus, On Thursday 14 March 2013 13:23:46 Magnus Damm wrote: > On Wed, Mar 13, 2013 at 9:58 PM, Laurent Pinchart wrote: > > On Wednesday 13 March 2013 20:32:03 Magnus Damm wrote: > >> gpio: Renesas R-Car GPIO driver update > >> > >> [PATCH 01/03] gpio: Renesas R-Car GPIO driver V2 > >> [PATCH 02/03] gpio: rcar: Use IRQCHIP_SET_TYPE_MASKED > >> [PATCH 03/03] gpio: rcar: Make use of devm functions > >> > >> This series updates the R-Car GPIO driver with various > >> changes kindly suggested by Laurent Pinchart. > >> > >> Patch [01/03] is a drop in replacement for V1 of the R-Car > >> GPIO driver. The flag in patch [02/03] is kept out of [01/03] > >> to avoid changing the behaviour of the driver between V1 and V2. > >> Also, devm support in [03/03] is kept out of [01/03] to make > >> sure back porting goes as smooth as possible. > > > > As I mentioned in a previous e-mail, all the devm_* functions used in > > 03/03 have been available since 2.6.30. Do you really need to port that > > driver to older kernels ? > > Well, it was earlier suggested to me that not using devm to begin with > is a safe way forward for back porting. Also, as the individual patch > shows, we save about 10 lines of code but add many more complex > dependencies. > > Simon, do you have any recommendation how for us regarding devm and > back porting? > > > Regarding 02/03, do you plan to squash it with 01/03 for the mainline > > submission ? > > Not unless someone puts a gun to my head. =) Of course, if a single > patch is really required then I will follow that, but I can't really > see why when we have a nice versioning control system that can help > our development in various ways. > > What is the reason behind you wanting to merge these patches? > > From my point of view, if any incremental patch was a bug fix then i > would of course request to fold it in, but in this case these are > feature patches that would be beneficial to keep as individual > commits. Keeping them separate allows us to bisect and also makes > partial back porting easier if needed. When submitting new drivers I usually try not to make the development history visible to mainline. It brings little additional value (beside possibly making backporting a bit easier, but in the devm_* case that shouldn't be a problem, unless Simon thinks otherwise) but adds review complexity, as reviewers need to validate the intermediate versions as well. More patches also mean more potential bisection breakages. In this case (assuming there would be no backporting issue) there's no need for mainline to see that we started with a version that didn't use devm_* and then modified the code. I see no added value in that. -- Regards, Laurent Pinchart -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 4/5] video:omap3isp:fix up ENOIOCTLCMD error handling
Hi Wanlong, Thanks for the patch. On Monday 27 August 2012 15:23:15 Wanlong Gao wrote: > At commit 07d106d0, Linus pointed out that ENOIOCTLCMD should be > translated as ENOTTY to user mode. > > Cc: Laurent Pinchart > Cc: Mauro Carvalho Chehab > Cc: linux-me...@vger.kernel.org > Signed-off-by: Wanlong Gao > --- > drivers/media/video/omap3isp/ispvideo.c | 10 +- > 1 file changed, 5 insertions(+), 5 deletions(-) > > diff --git a/drivers/media/video/omap3isp/ispvideo.c > b/drivers/media/video/omap3isp/ispvideo.c index b37379d..2dd982e 100644 > --- a/drivers/media/video/omap3isp/ispvideo.c > +++ b/drivers/media/video/omap3isp/ispvideo.c > @@ -337,7 +337,7 @@ __isp_video_get_format(struct isp_video *video, struct > v4l2_format *format) fmt.which = V4L2_SUBDEV_FORMAT_ACTIVE; > ret = v4l2_subdev_call(subdev, pad, get_fmt, NULL, &fmt); > if (ret == -ENOIOCTLCMD) > - ret = -EINVAL; > + ret = -ENOTTY; I don't think this location should be changed. __isp_video_get_format() is called by isp_video_check_format() only, which in turn is called by isp_video_streamon() only. A failure to retrieve the format in __isp_video_get_format() does not really mean the VIDIOC_STREAMON is not supported. I'll apply hunks 2 to 5 and drop hunk 1 if that's fine with you. > > mutex_unlock(&video->mutex); > > @@ -723,7 +723,7 @@ isp_video_try_format(struct file *file, void *fh, struct > v4l2_format *format) fmt.which = V4L2_SUBDEV_FORMAT_ACTIVE; > ret = v4l2_subdev_call(subdev, pad, get_fmt, NULL, &fmt); > if (ret) > - return ret == -ENOIOCTLCMD ? -EINVAL : ret; > + return ret == -ENOIOCTLCMD ? -ENOTTY : ret; > > isp_video_mbus_to_pix(video, &fmt.format, &format->fmt.pix); > return 0; > @@ -744,7 +744,7 @@ isp_video_cropcap(struct file *file, void *fh, struct > v4l2_cropcap *cropcap) ret = v4l2_subdev_call(subdev, video, cropcap, > cropcap); > mutex_unlock(&video->mutex); > > - return ret == -ENOIOCTLCMD ? -EINVAL : ret; > + return ret == -ENOIOCTLCMD ? -ENOTTY : ret; > } > > static int > @@ -771,7 +771,7 @@ isp_video_get_crop(struct file *file, void *fh, struct > v4l2_crop *crop) format.which = V4L2_SUBDEV_FORMAT_ACTIVE; > ret = v4l2_subdev_call(subdev, pad, get_fmt, NULL, &format); > if (ret < 0) > - return ret == -ENOIOCTLCMD ? -EINVAL : ret; > + return ret == -ENOIOCTLCMD ? -ENOTTY : ret; > > crop->c.left = 0; > crop->c.top = 0; > @@ -796,7 +796,7 @@ isp_video_set_crop(struct file *file, void *fh, struct > v4l2_crop *crop) ret = v4l2_subdev_call(subdev, video, s_crop, crop); > mutex_unlock(&video->mutex); > > - return ret == -ENOIOCTLCMD ? -EINVAL : ret; > + return ret == -ENOIOCTLCMD ? -ENOTTY : ret; > } > > static int -- Regards, Laurent Pinchart -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2] media: v4l2-ctrls: add control for test pattern
Hi Sakari,
On Sunday 09 September 2012 10:40:17 Sakari Ailus wrote:
> On Sat, Sep 08, 2012 at 01:11:04PM +0200, Hans Verkuil wrote:
> > On Fri September 7 2012 20:20:51 Sakari Ailus wrote:
> > > Prabhakar Lad wrote:
> > > > From: Lad, Prabhakar
> > > >
> > > > add V4L2_CID_TEST_PATTERN of type menu, which determines
> > > > the internal test pattern selected by the device.
> > > >
> > > > Signed-off-by: Lad, Prabhakar
> > > > Signed-off-by: Manjunath Hadli
> > > > Cc: Sakari Ailus
> > > > Cc: Hans Verkuil
> > > > Cc: Laurent Pinchart
> > > > Cc: Mauro Carvalho Chehab
> > > > Cc: Sylwester Nawrocki
> > > > Cc: Hans de Goede
> > > > Cc: Kyungmin Park
> > > > Cc: Rob Landley
> > > > ---
> > > > This patches has one checkpatch warning for line over
> > > > 80 characters altough it can be avoided I have kept it
> > > > for consistency.
> > > >
> > > > Changes for v2:
> > > > 1: Included display devices in the description for test pattern
> > > > as pointed by Hans.
> > > >
> > > > 2: In the menu replaced 'Test Pattern Disabled' by 'Disabled' as
> > > > pointed by Sylwester.
> > > >
> > > > 3: Removed the test patterns from menu as the are hardware specific
> > > > as pointed by Sakari.
> > > >
> > > > Documentation/DocBook/media/v4l/controls.xml | 20
> > > > drivers/media/v4l2-core/v4l2-ctrls.c |8
> > > > include/linux/videodev2.h|4
> > > > 3 files changed, 32 insertions(+), 0 deletions(-)
> > > >
> > > > diff --git a/Documentation/DocBook/media/v4l/controls.xml
> > > > b/Documentation/DocBook/media/v4l/controls.xml index ad873ea..173934e
> > > > 100644
> > > > --- a/Documentation/DocBook/media/v4l/controls.xml
> > > > +++ b/Documentation/DocBook/media/v4l/controls.xml
> > > > @@ -4311,6 +4311,26 @@ interface and may change in the future.
> > > >
> > > >
> > > >
> > > > +
> > > > +> > > spanname="id">V4L2_CID_TEST_PATTERN +
> > > >
> > > > menu
> > > > +
> > > > +
> > > > +The Capture/Display/Sensors have
> > > > the
> > > > capability
> > > > + to generate internal test patterns and this are hardware
> > > > specific. This
> > > > + test patterns are used to test a device is properly working
> > > > and
> > > > can generate
> > > > + the desired waveforms that
> > > > it supports.
> > > > +
> > > > +
> > > > +
> > > > +
> > > > +
> > > > +
> > > > V4L2_TEST_PATTERN_DISABLED
> > > > + Test pattern generation is disabled
> > > > +
> > > > +
> > > > +
> > > > +
> > > >
> > > >
> > > >
> > > > diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c
> > > > b/drivers/media/v4l2-core/v4l2-ctrls.c index 8f2f40b..d731422 100644
> > > > --- a/drivers/media/v4l2-core/v4l2-ctrls.c
> > > > +++ b/drivers/media/v4l2-core/v4l2-ctrls.c
> > > > @@ -430,6 +430,10 @@ const char * const *v4l2_ctrl_get_menu(u32 id)
> > > > "Advanced",
> > > > NULL,
> > > > };
> > > > + static const char * const test_pattern[] = {
> > > > + "Disabled",
> > > > + NULL,
> > > > + };
> > > >
> > > > switch (id) {
> > > > case V4L2_CID_MPEG_AUDIO_SAMPLING_FREQ:
> > > > @@ -509,6 +513,8 @@ const char * const *v4l2_ctrl_get_menu(u32 id)
> > > > return jpeg_chroma_subsampling;
> > > > case V4L2_CID_DPCM_PREDICTOR:
> > > > return dpcm_predictor;
> > > > + case V4L2_CID_TEST_PATTERN:
> > > > + return te
Re: [PATCH v4] media: v4l2-ctrls: add control for dpcm predictor
Hi Sakari, On Friday 07 September 2012 21:46:44 Sakari Ailus wrote: > > Could you replace the above with this text (with appropriate indentation > etc.) while keeping the reference to Wikipedia? > > --8<-- > Differential pulse-code modulation (DPCM) compression can be used to > compress the samples into fewer bits than they would otherwise require. > This is done by calculating the difference between consecutive samples > and outputting the difference which in average is much smaller than the > values of the samples themselves since there is generally lots of > correlation between adjacent pixels. In decompression the original > samples are reconstructed. The process isn't lossless as the encoded > sample size in bits is less than the original. > > Formats using DPCM compression include linkend="pixfmt-srggb10dpcm8" />. > > This control is used to select the predictor used to encode the samples. If I remember correctly this control will be used on the receiver side on DaVinci, to decode pixels not encode them. How is the predictor used in that case ? Must it match the predictor used on the encoding side ? If so I expect documentation to be available somewhere. The OMAP3 ISP supports both DPCM encoding and decoding, and documents the predictors as "- The simple predictor This predictor uses only the previous same color component value as a prediction value. Therefore, only two-pixel memory is required. - The advanced predictor This predictor uses four previous pixel values, when the prediction value is evaluated. This means that also the other color component values are used, when the prediction value has been defined." It also states the the simple predictor is preferred for 10-8-10 conversion, and the advanced predictor for 10-7-10 and 10-6-10 conversion. > The main difference between the simple and the advanced predictors is > image quality, with advanced predictor supposed to produce better > quality images as a result. Simple predictor can be used e.g. for > testing purposes. > --8<-- -- Regards, Laurent Pinchart -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v3] media: v4l2-ctrl: add a helper function to modify the menu
Hi Prabhakar,
Thanks for the patch.
On Tuesday 11 September 2012 19:53:38 Prabhakar Lad wrote:
> From: Lad, Prabhakar
>
> Add a helper function to modify the menu, max and default value
> to set.
>
> Signed-off-by: Lad, Prabhakar
> Signed-off-by: Manjunath Hadli
> Cc: Hans Verkuil
> Cc: Sakari Ailus
> Cc: Sylwester Nawrocki
> Cc: Laurent Pinchart
> Cc: Mauro Carvalho Chehab
> Cc: Hans de Goede
> Cc: Kyungmin Park
> Cc: Guennadi Liakhovetski
> Cc: Rob Landley
> ---
> Changes for v3:
> 1: Fixed style/grammer issues as pointed by Hans.
>Thanks Hans for providing the description.
>
> Changes for v2:
> 1: Fixed review comments from Hans, to have return type as
>void, add WARN_ON() for fail conditions, allow this fucntion
>to modify the menu of custom controls.
>
> Documentation/video4linux/v4l2-controls.txt | 29
> drivers/media/v4l2-core/v4l2-ctrls.c| 17 +++
> include/media/v4l2-ctrls.h | 11 ++
> 3 files changed, 57 insertions(+), 0 deletions(-)
>
> diff --git a/Documentation/video4linux/v4l2-controls.txt
> b/Documentation/video4linux/v4l2-controls.txt index 43da22b..01d0a82 100644
> --- a/Documentation/video4linux/v4l2-controls.txt
> +++ b/Documentation/video4linux/v4l2-controls.txt
> @@ -367,6 +367,35 @@ it to 0 means that all menu items are supported.
> You set this mask either through the v4l2_ctrl_config struct for a custom
> control, or by calling v4l2_ctrl_new_std_menu().
>
> +There are situations where menu items may be device specific. In such cases
> the
> +framework provides a helper function to change the menu:
> +
> +void v4l2_ctrl_modify_menu(struct v4l2_ctrl *ctrl, const char * const
> *qmenu,
> + s32 max, u32 menu_skip_mask, s32 def);
Sorry if this is a stupid question, but wouldn't it be better to add a
function to create a custom menu instead of modifying it afterwards ?
> +
> +A good example is the test pattern control for capture/display/sensors
> devices
> +that have the capability to generate test patterns. These test patterns are
> +hardware specific, so the contents of the menu will vary from device to
> device.
> +
> +This helper function is used to modify the menu, max, mask and the default
> +value of the control.
> +
> +Example:
> +
> + static const char * const test_pattern[] = {
> + "Disabled",
> + "Vertical Bars",
> + "Solid Black",
> + "Solid White",
> + NULL,
> + };
> + struct v4l2_ctrl *test_pattern_ctrl =
> + v4l2_ctrl_new_std_menu(&foo->ctrl_handler, &foo_ctrl_ops,
> + V4L2_CID_TEST_PATTERN, V4L2_TEST_PATTERN_DISABLED, 0,
> + V4L2_TEST_PATTERN_DISABLED);
> +
> + v4l2_ctrl_modify_menu(test_pattern_ctrl, test_pattern, 3, 0,
> + V4L2_TEST_PATTERN_DISABLED);
>
> Custom Controls
> ===
--
Regards,
Laurent Pinchart
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v5] media: mt9p031/mt9t001/mt9v032: use V4L2_CID_TEST_PATTERN for test pattern control
Hi Prabhakar, On Wednesday 03 October 2012 19:23:05 Prabhakar wrote: > From: Lad, Prabhakar > > V4L2_CID_TEST_PATTERN is now a standard control. > This patch replaces the user defined control for test > pattern to make use of standard control V4L2_CID_TEST_PATTERN. > > Signed-off-by: Lad, Prabhakar > Signed-off-by: Manjunath Hadli > Acked-by: Laurent Pinchart > Cc: Sakari Ailus > Cc: Paul Gortmaker > Cc: Jean Delvare Should I push this patch through my tree ? If so I'll wait until the V4L2_CID_TEST_PATTERN control patch hits Mauro's tree. -- Regards, Laurent Pinchart -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 0/2] USB webcam gadget driver fix for linux-next
Hi, The first patch in this series should fix the UVC gadget compilation errors in linux-next. The second patch should make sure I'll be notified next time a patch tries to break it :-) Both patches are based on Linus' tree. Laurent Pinchart (2): usb: gadget: Make webcam gadget select USB_LIBCOMPOSITE MAINTAINERS: Add maintainer entry for the USB webcam gadget MAINTAINERS|7 +++ drivers/usb/gadget/Kconfig |1 + 2 files changed, 8 insertions(+), 0 deletions(-) -- Regards, Laurent Pinchart -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 1/2] usb: gadget: Make webcam gadget select USB_LIBCOMPOSITE
Composite gadget support is now available as a library instead of being built with each gadget. Composite drivers need to select USB_LIBCOMPOSITE. Signed-off-by: Laurent Pinchart --- drivers/usb/gadget/Kconfig |1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/drivers/usb/gadget/Kconfig b/drivers/usb/gadget/Kconfig index dfb51a4..e0ff51b 100644 --- a/drivers/usb/gadget/Kconfig +++ b/drivers/usb/gadget/Kconfig @@ -952,6 +952,7 @@ endif config USB_G_WEBCAM tristate "USB Webcam Gadget" depends on VIDEO_DEV + select USB_LIBCOMPOSITE help The Webcam Gadget acts as a composite USB Audio and Video Class device. It provides a userspace API to process UVC control requests -- 1.7.8.6 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 2/2] MAINTAINERS: Add maintainer entry for the USB webcam gadget
Signed-off-by: Laurent Pinchart --- MAINTAINERS |7 +++ 1 files changed, 7 insertions(+), 0 deletions(-) diff --git a/MAINTAINERS b/MAINTAINERS index d919e3d..b118b23 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -7392,6 +7392,13 @@ S: Maintained F: Documentation/video4linux/w9968cf.txt F: drivers/media/video/w996* +USB WEBCAM GADGET +M: Laurent Pinchart +L: linux-...@vger.kernel.org +S: Maintained +F: drivers/usb/gadget/*uvc*.c +F: drivers/usb/gadget/webcam.c + USB WIRELESS RNDIS DRIVER (rndis_wlan) M: Jussi Kivilinna L: linux-wirel...@vger.kernel.org -- 1.7.8.6 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH RFC] media: v4l2-ctrl: Add digital gain controls
Hi Prabhakar, Thanks for the patch. On Monday 01 October 2012 19:33:39 Prabhakar wrote: > From: Lad, Prabhakar > > add support for per color component digital gain controls > and also their corresponding offset. > > Signed-off-by: Lad, Prabhakar > Signed-off-by: Manjunath Hadli > Cc: Sakari Ailus > Cc: Laurent Pinchart > Cc: Guennadi Liakhovetski > Cc: Sylwester Nawrocki > Cc: Hans Verkuil > Cc: Hans de Goede hdego...@redhat.com > Cc: Chris MacGregor ch...@cybermato.com > Cc: Rob Landley > Cc: Mauro Carvalho Chehab > --- > Documentation/DocBook/media/v4l/controls.xml | 54 +++ > drivers/media/v4l2-core/v4l2-ctrls.c | 11 + > include/linux/v4l2-controls.h| 11 + > 3 files changed, 76 insertions(+), 0 deletions(-) > > diff --git a/Documentation/DocBook/media/v4l/controls.xml > b/Documentation/DocBook/media/v4l/controls.xml index a84a08c..9bf54f3 > 100644 > --- a/Documentation/DocBook/media/v4l/controls.xml > +++ b/Documentation/DocBook/media/v4l/controls.xml > @@ -4277,6 +4277,60 @@ interface and may change in the future. > specific test patterns can be used to test if a device is working > properly. > > + > + V4L2_CID_GAIN_RED > + integer > + > + > + spanname="id">V4L2_CID_GAIN_GREEN_RED + > integer > + > + > + spanname="id">V4L2_CID_GAIN_GREEN_BLUE + > integer > + > + > + V4L2_CID_GAIN_BLUE > + integer > + > + > + spanname="id">V4L2_CID_GAIN_GREEN > + integer > + > + > + Some capture/sensor devices have > + the capability to set per color component digital gain values. > + > + > + V4L2_CID_GAIN_OFFSET > + integer > + > + > + V4L2_CID_BLUE_OFFSET > + integer > + > + > + spanname="id">V4L2_CID_RED_OFFSET > + integer > + > + > + spanname="id">V4L2_CID_GREEN_OFFSET + > integer > + > + > + spanname="id">V4L2_CID_GREEN_RED_OFFSET + > > integer > + > + > + spanname="id">V4L2_CID_GREEN_BLUE_OFFSET + > > integer > + > + > + Some capture/sensor devices have the > + capability to set per color component digital gain offset values. > + V4L2_CID_GAIN_OFFSET is the global gain offset and the rest are per > + color component gain offsets. > + > > > > diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c > b/drivers/media/v4l2-core/v4l2-ctrls.c index 93cd0a4..02cc1d2 100644 > --- a/drivers/media/v4l2-core/v4l2-ctrls.c > +++ b/drivers/media/v4l2-core/v4l2-ctrls.c > @@ -750,6 +750,17 @@ const char *v4l2_ctrl_get_name(u32 id) > case V4L2_CID_LINK_FREQ:return "Link Frequency"; > case V4L2_CID_PIXEL_RATE: return "Pixel Rate"; > case V4L2_CID_TEST_PATTERN: return "Test Pattern"; > + case V4L2_CID_GAIN_RED: return "Digital Gain Red"; > + case V4L2_CID_GAIN_GREEN_RED: return "Digital Gain Green Red"; > + case V4L2_CID_GAIN_GREEN_BLUE: return "Digital Gain Green > Blue"; > + case V4L2_CID_GAIN_BLUE:return "Digital Gain Blue"; > + case V4L2_CID_GAIN_GREEN: return "Digital Gain Green"; > + case V4L2_CID_GAIN_OFFSET: return "Digital Gain Offset"; > + case V4L2_CID_BLUE_OFFSET: return "Digital Gain Blue > Offset"; > + case V4L2_CID_RED_OFFSET: return "Digital Gain Red > Offset"; > + case V4L2_CID_GREEN_OFFSET: return "Digital Gain Green > Offset"; > + case V4L2_CID_GREEN_RED_OFFSET: return "Digital Gain Green Red Offset"; > + case V4L2_CID_GREEN_BLUE_OFFSET:return "Digital Gain Green Blue Offset"; I would remove the mention of "Digital" here, as those controls can be used for analog gains as well. Same above in the documentation. > /* DV controls */ > case V4L2_CID_DV_CLASS: return "Digital Video Controls"; > diff --git a/include/linux/v4l2-controls.h b/include/linux/v4l2-controls.h > index e1b3680..087596d 100644 > --- a/include/linux/v
Re: [PATCH 01/23] uvc: Replace memcpy with struct assignment
Hi Ezequiel, Thanks for the patch. On Tuesday 23 October 2012 16:57:04 Ezequiel Garcia wrote: > This kind of memcpy() is error-prone. Its replacement with a struct > assignment is prefered because it's type-safe and much easier to read. > > Found by coccinelle. Hand patched and reviewed. > Tested by compilation only. This looks good, but there's one more memcpy that can be replaced by a direct structure assignment in uvc_ctrl_add_info() (drivers/media/usb/uvc/uvc_ctrl.c). You might want to check why it hasn't been caught by the semantic patch. > A simplified version of the semantic match that finds this problem is as > follows: (http://coccinelle.lip6.fr/) > > // > @@ > identifier struct_name; > struct struct_name to; > struct struct_name from; > expression E; > @@ > -memcpy(&(to), &(from), E); > +to = from; > // > > Cc: Laurent Pinchart > Signed-off-by: Peter Senna Tschudin > Signed-off-by: Ezequiel Garcia > --- > drivers/media/usb/uvc/uvc_v4l2.c |6 +++--- > 1 files changed, 3 insertions(+), 3 deletions(-) > > diff --git a/drivers/media/usb/uvc/uvc_v4l2.c > b/drivers/media/usb/uvc/uvc_v4l2.c index f00db30..4fc8737 100644 > --- a/drivers/media/usb/uvc/uvc_v4l2.c > +++ b/drivers/media/usb/uvc/uvc_v4l2.c > @@ -314,7 +314,7 @@ static int uvc_v4l2_set_format(struct uvc_streaming > *stream, goto done; > } > > - memcpy(&stream->ctrl, &probe, sizeof probe); > + stream->ctrl = probe; > stream->cur_format = format; > stream->cur_frame = frame; > > @@ -386,7 +386,7 @@ static int uvc_v4l2_set_streamparm(struct uvc_streaming > *stream, return -EBUSY; > } > > - memcpy(&probe, &stream->ctrl, sizeof probe); > + probe = stream->ctrl; > probe.dwFrameInterval = > uvc_try_frame_interval(stream->cur_frame, interval); > > @@ -397,7 +397,7 @@ static int uvc_v4l2_set_streamparm(struct uvc_streaming > *stream, return ret; > } > > - memcpy(&stream->ctrl, &probe, sizeof probe); > + stream->ctrl = probe; > mutex_unlock(&stream->mutex); > > /* Return the actual frame period. */ -- Regards, Laurent Pinchart -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/1] usb: gadget: Don't attempt to dequeue requests for a disabled USB endpoint on Freescale hardware
Hi Simon, On Monday 22 October 2012 12:49:51 Simon Haggett wrote: > On 22/10/12 11:47, Laurent Pinchart wrote: > > On Monday 22 October 2012 03:33:19 Li Yang-R58472 wrote: > >> On Saturday, October 20, 2012 1:37 AM Felipe Balbi wrote: > >>> On Fri, Oct 19, 2012 at 06:19:26PM +0100, Simon Haggett wrote: > >>>> Some gadget drivers may attempt to dequeue requests for an endpoint > >>>> that has already been disabled. For example, in the UVC gadget driver, > >>>> uvc_function_set_alt() will call usb_ep_disable() when alt setting 0 > >>>> is selected. When the userspace application subsequently issues the > >>>> VIDIOC_STREAMOFF ioctl, uvc_video_enable() invokes usb_ep_dequeue() to > >>> > >>> ensure that all requests have been cancelled. > >>> > >>> bug is on uvc gadget, then. Laurent ? > > > > We've discussed this topic a couple of months before. I believe that's not > > a bug. > > > > http://68.183.106.108/lists/linux-usb/msg68869.html > > > >>> Also, fsl should be removed from the tree, I'm trying to persuade iMX > >>> folks to use drivers/usb/chipidea instead. > >> > >> Besides the iMX usage, the driver is also being used by many Freescale > >> PowerPC/Coldfire SoCs. I agree that it's ideal to move to a common > >> driver. > >> But it is a large task to make the chipidea driver works for all the > >> hardware that fsl_udc had supported and been tested on. > >> > >>>> For the Freescale High Speed Dual-Role USB controller, > >>>> fsl_ep_dequeue() provides the implementation of usb_ep_dequeue(). If > >>>> this is called for a disabled endpoint, a kernel oops will occur when > >>> > >>> the ep->ep.desc field is dereferenced (by ep_index()). > >>> > >>>> fsl_ep_disable() sets this field to NULL, as well as deleting all > >>>> pending requests for the endpoint. > >>>> > >>>> This patch adds an additional check to fsl_ep_dequeue() to ensure that > >>>> the endpoint has not already been disabled before attempting to dequeue > >>> > >>> requests. > >>> > >>>> Signed-off-by: Simon Haggett > >>>> --- > >>>> > >>>> drivers/usb/gadget/fsl_udc_core.c |5 - > >>>> 1 files changed, 4 insertions(+), 1 deletions(-) > >>>> > >>>> diff --git a/drivers/usb/gadget/fsl_udc_core.c > >>>> b/drivers/usb/gadget/fsl_udc_core.c > >>>> index 6ae70cb..acd513b 100644 > >>>> --- a/drivers/usb/gadget/fsl_udc_core.c > >>>> +++ b/drivers/usb/gadget/fsl_udc_core.c > >>>> @@ -955,7 +955,10 @@ static int fsl_ep_dequeue(struct usb_ep *_ep, > >>> > >>> struct usb_request *_req) > >>> > >>>> int ep_num, stopped, ret = 0; > >>>> u32 epctrl; > >>>> > >>>> -if (!_ep || !_req) > >>>> +/* Ensure that the ep and request are valid, and the ep is not > >>>> + * disabled > >>>> + */ > >>>> +if (!_ep || !_req || !ep->ep.desc) > >>>> > >>>> return -EINVAL; > > > > Shouldn't that last check be done with a lock taken ? > > I had presumed a lock wasn't necessary because ep->ep.desc is only set > to NULL by fsl_ep_disable() which, since it is called by > usb_ep_disable(), should only be invoked when no other task is using the > endpoint (according to include/linux/usb/gadget.h). Furthermore, the > chipidea UDC driver does check the equivalent of this field is not NULL > without taking a lock (ep_dequeue() in drivers/usb/chipidea/udc.c). > > However, it is possible that I'm misunderstanding something here, so > apologies if I am. I might be wrong as well :-) I just wanted to point out something that appeared to me as a possible issue. > >>>> spin_lock_irqsave(&ep->udc->lock, flags); -- Regards, Laurent Pinchart -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/1] usb: gadget: Don't attempt to dequeue requests for a disabled USB endpoint on Freescale hardware
Hi Felipe, On Monday 22 October 2012 13:56:01 Felipe Balbi wrote: > On Mon, Oct 22, 2012 at 12:47:21PM +0200, Laurent Pinchart wrote: > > On Monday 22 October 2012 03:33:19 Li Yang-R58472 wrote: > > > On Saturday, October 20, 2012 1:37 AM Felipe Balbi wrote: > > > > On Fri, Oct 19, 2012 at 06:19:26PM +0100, Simon Haggett wrote: > > > > > Some gadget drivers may attempt to dequeue requests for an endpoint > > > > > that has already been disabled. For example, in the UVC gadget > > > > > driver, uvc_function_set_alt() will call usb_ep_disable() when alt > > > > > setting 0 is selected. When the userspace application subsequently > > > > > issues the VIDIOC_STREAMOFF ioctl, uvc_video_enable() invokes > > > > > usb_ep_dequeue() to ensure that all requests have been cancelled. > > > > > > > > bug is on uvc gadget, then. Laurent ? > > > > We've discussed this topic a couple of months before. I believe that's not > > a bug. > > > > http://68.183.106.108/lists/linux-usb/msg68869.html > > fair enough :-) > > That's a different case, however. At the link above we're discussing > dequeueing a request which is already being dequeued. $SUBJECT is trying > to fix dequeueing of a request for an endpoint which isn't even enabled. You've got a point there :-) That's a different case indeed, I'm open to (at least evaluating) a fix in the UVC gadget driver if you think that's better. -- Regards, Laurent Pinchart signature.asc Description: This is a digitally signed message part.
Re: [PATCH v2] media: davinci: vpbe: fix build warning
Hi Prabhakar,
Thanks for the patch.
On Tuesday 23 October 2012 18:54:37 Prabhakar Lad wrote:
> From: Lad, Prabhakar
>
> Warnings were generated because of the following commit changed data type
> for address pointer
>
> 195bbca ARM: 7500/1: io: avoid writeback addressing modes for __raw_
> accessors add __iomem annotation to fix following warnings
>
> drivers/media/platform/davinci/vpbe_osd.c: In function ‘osd_read’:
> drivers/media/platform/davinci/vpbe_osd.c:49:2: warning: passing
> argument 1 of ‘__raw_readl’ makes pointer from integer without a cast
> [enabled by default] arch/arm/include/asm/io.h:104:19: note: expected
> ‘const volatile
> void *’ but argument is of type ‘long unsigned int’
Please add a sentence here that explains what your patch does. Something like
"Store the ioremap_nocache() returned address in a void __iomem * instead of a
unsigned long."
> Signed-off-by: Lad, Prabhakar
> Signed-off-by: Manjunath Hadli
> Cc: Laurent Pinchart
> Cc: Mauro Carvalho Chehab
Acked-by: Laurent Pinchart
> ---
> Changes for v2:
> 1: Made the base addr to void __iomem * instead of long unsigned,
> as pointed by Laurent.
>
> drivers/media/platform/davinci/vpbe_osd.c |9 -
> include/media/davinci/vpbe_osd.h |2 +-
> 2 files changed, 5 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/media/platform/davinci/vpbe_osd.c
> b/drivers/media/platform/davinci/vpbe_osd.c index bba299d..707f243 100644
> --- a/drivers/media/platform/davinci/vpbe_osd.c
> +++ b/drivers/media/platform/davinci/vpbe_osd.c
> @@ -62,7 +62,7 @@ static inline u32 osd_set(struct osd_state *sd, u32 mask,
> u32 offset) {
> struct osd_state *osd = sd;
>
> - u32 addr = osd->osd_base + offset;
> + void __iomem *addr = osd->osd_base + offset;
> u32 val = readl(addr) | mask;
>
> writel(val, addr);
> @@ -74,7 +74,7 @@ static inline u32 osd_clear(struct osd_state *sd, u32
> mask, u32 offset) {
> struct osd_state *osd = sd;
>
> - u32 addr = osd->osd_base + offset;
> + void __iomem *addr = osd->osd_base + offset;
> u32 val = readl(addr) & ~mask;
>
> writel(val, addr);
> @@ -87,7 +87,7 @@ static inline u32 osd_modify(struct osd_state *sd, u32
> mask, u32 val, {
> struct osd_state *osd = sd;
>
> - u32 addr = osd->osd_base + offset;
> + void __iomem *addr = osd->osd_base + offset;
> u32 new_val = (readl(addr) & ~mask) | (val & mask);
>
> writel(new_val, addr);
> @@ -1559,8 +1559,7 @@ static int osd_probe(struct platform_device *pdev)
> ret = -ENODEV;
> goto free_mem;
> }
> - osd->osd_base = (unsigned long)ioremap_nocache(res->start,
> - osd->osd_size);
> + osd->osd_base = ioremap_nocache(res->start, osd->osd_size);
> if (!osd->osd_base) {
> dev_err(osd->dev, "Unable to map the OSD region\n");
> ret = -ENODEV;
> diff --git a/include/media/davinci/vpbe_osd.h
> b/include/media/davinci/vpbe_osd.h index d7e397a..5ab0d8d 100644
> --- a/include/media/davinci/vpbe_osd.h
> +++ b/include/media/davinci/vpbe_osd.h
> @@ -357,7 +357,7 @@ struct osd_state {
> spinlock_t lock;
> struct device *dev;
> dma_addr_t osd_base_phys;
> - unsigned long osd_base;
> + void __iomem *osd_base;
> unsigned long osd_size;
> /* 1-->the isr will toggle the VID0 ping-pong buffer */
> int pingpong;
--
Regards,
Laurent Pinchart
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 4/4] fbdev: sh_mobile_lcdc: use dma_mmap_coherent if available
Hi Eiraku-san, On Thursday 26 July 2012 20:13:11 Hideki EIRAKU wrote: > fb_mmap() implemented in fbmem.c uses smem_start as the physical > address of the frame buffer. In the sh_mobile_lcdc driver, the > smem_start is a dma_addr_t that is not a physical address when IOMMU is > enabled. dma_mmap_coherent() maps the address correctly. It is > available on ARM platforms. > > Signed-off-by: Hideki EIRAKU Acked-by: Laurent Pinchart -- Regards, Laurent Pinchart -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] [media] davinci: vpfe: Add documentation
Hi Manjunath,
On Friday 27 July 2012 05:49:24 Hadli, Manjunath wrote:
> On Thu, Jul 26, 2012 at 05:55:31, Laurent Pinchart wrote:
> > On Tuesday 17 July 2012 10:43:54 Hadli, Manjunath wrote:
> > > On Sun, Jul 15, 2012 at 18:16:25, Laurent Pinchart wrote:
> > > > On Wednesday 11 July 2012 21:09:26 Manjunath Hadli wrote:
> > > > > Add documentation on the Davinci VPFE driver. Document the subdevs,
> > > > > and private IOTCLs the driver implements
> > > > >
> > > > > Signed-off-by: Manjunath Hadli
> > > > > Signed-off-by: Lad, Prabhakar
> >
> > [snip]
> >
> > > > > +Private IOCTLs
> > > > > +==
> > > > > +
> > > > > +The Davinci Video processing Front End (VPFE) driver supports
> > > > > standard V4L2
> > > > > +IOCTLs and controls where possible and practical. Much of the
> > > > > functions provided
> > > > > +by the VPFE, however, does not fall under the standard IOCTLs.
> > > > > +
> > > > > +In general, there is a private ioctl for configuring each of the
> > > > > blocks
> > > > > +containing hardware-dependent functions.
> > > > > +
> > > > > +The following private IOCTLs are supported:
> > > > > +
> > > > > +1: IOCTL: PREV_S_PARAM/PREV_G_PARAM
> > > > > +Description:
> > > > > + Sets/Gets the parameters required by the previewer module
> > > > > +Parameter:
> > > > > + /**
> > > > > + * struct prev_module_param- structure to configure preview
> > > > > modules
> > > > > + * @version: Version of the preview module
> > > >
> > > > Who is responsible for filling this field, the application or the
> > > > driver ?
> > >
> > > The application is responsible for filling this info. He would enumerate
> > > the capabilities first and set them using S_PARAM/G_PARAM.
> >
> > And what's the point of the application setting the version field ? How
> > does the driver use it ?
>
> The version may not be required. Will remove it.
>
> > > > > + * @len: Length of the module config structure
> > > > > + * @module_id: Module id
> > > > > + * @param: pointer to module config parameter.
> > > >
> > > > What is module_id for ? What does param point to ?
> > >
> > > There are a lot of tiny modules in the previewer/resizer which are
> > > enumerated as individual modules. The param points to the parameter set
> > > that the module expects to be set.
> >
> > Why don't you implement something similar to
> > VPFE_CMD_S_CCDC_RAW_PARAMS/VPFE_CMD_G_CCDC_RAW_PARAMS instead ?
>
> I feel if we implement direct IOCTLS there might be many of them. To make
> sure than independent of the number of internal modules present, having the
> same IOCTL used for all modules is a good idea.
You can set several parameters using a single ioctl, much like
VPFE_CMD_S_CCDC_RAW_PARAMS does. You don't need one ioctl per parameter.
PREV_ENUM_CAP, PREV_[GS]_PARAM and PREV_[GS]_CONFIG are essentially
reinventing V4L2 controls, and I don't think that's a good idea.
> > > > > + */
> > > > > + struct prev_module_param {
> > > > > + char version[IMP_MAX_NAME_SIZE];
> > > >
> > > > Is there a need to express the version as a string instead of an
> > > > integer ?
> > >
> > > It could be integer. It is generally a fixed point num, and easy to read
> > > it as a string than an integer. Can I keep it as a string?
> >
> > Let's first decide whether a version field is needed at all :-)
>
> Will remove.
>
> > > > > + unsigned short len;
> > > > > + unsigned short module_id;
> > > > > + void *param;
> > > > > + };
> > > > > +
> > > > > +2: IOCTL: PREV_S_CONFIG/PREV_G_CONFIG
> > > > > +Description:
> > > > > + Sets/Gets the configuration required by the previewer channel
> > > > > +Parameter:
> > > > > + /**
> > > > > + * struct prev_channel_config - structure for configuring the
> > > > > previewer
> > > > > channel
> > > > > + * @len: Length of the user configu
Re: [PATCH 1/1] [media] uvcvideo: Add 10,12bit and alternate 8bit greyscale
Hi Stefan,
Thanks for the patch.
On Saturday 28 July 2012 18:49:14 Stefan Muenzel wrote:
> Some cameras support 10bit and 12bit greyscale, or use the alternate "Y8
> " FOURCC for 8bit greyscale. Add support for these.
Could you please tell me which camera(s) use those formats ?
> Tested on a 12bit camera.
>
> Signed-off-by: Stefan Muenzel
> ---
> drivers/media/video/uvc/uvc_driver.c | 19 +--
> drivers/media/video/uvc/uvcvideo.h |9 +
> 2 files changed, 26 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/media/video/uvc/uvc_driver.c
> b/drivers/media/video/uvc/uvc_driver.c index 1d13172..11db262 100644
> --- a/drivers/media/video/uvc/uvc_driver.c
> +++ b/drivers/media/video/uvc/uvc_driver.c
> @@ -95,12 +95,27 @@ static struct uvc_format_desc uvc_fmts[] = {
> .fcc= V4L2_PIX_FMT_UYVY,
> },
> {
> - .name = "Greyscale (8-bit)",
> + .name = "Greyscale 8-bit (Y800)",
> .guid = UVC_GUID_FORMAT_Y800,
> .fcc= V4L2_PIX_FMT_GREY,
> },
> {
> - .name = "Greyscale (16-bit)",
> + .name = "Greyscale 8-bit (Y8 )",
> + .guid = UVC_GUID_FORMAT_Y8,
> + .fcc= V4L2_PIX_FMT_GREY,
> + },
> + {
> + .name = "Greyscale 10-bit (Y10 )",
> + .guid = UVC_GUID_FORMAT_Y10,
> + .fcc= V4L2_PIX_FMT_Y10,
> + },
> + {
> + .name = "Greyscale 12-bit (Y12 )",
> + .guid = UVC_GUID_FORMAT_Y12,
> + .fcc= V4L2_PIX_FMT_Y12,
> + },
> + {
> + .name = "Greyscale 16-bit (Y16 )",
> .guid = UVC_GUID_FORMAT_Y16,
> .fcc= V4L2_PIX_FMT_Y16,
> },
> diff --git a/drivers/media/video/uvc/uvcvideo.h
> b/drivers/media/video/uvc/uvcvideo.h index 7c3d082..3764040 100644
> --- a/drivers/media/video/uvc/uvcvideo.h
> +++ b/drivers/media/video/uvc/uvcvideo.h
> @@ -79,6 +79,15 @@
> #define UVC_GUID_FORMAT_Y800 \
> { 'Y', '8', '0', '0', 0x00, 0x00, 0x10, 0x00, \
>0x80, 0x00, 0x00, 0xaa, 0x00, 0x38, 0x9b, 0x71}
> +#define UVC_GUID_FORMAT_Y8 \
> + { 'Y', '8', ' ', ' ', 0x00, 0x00, 0x10, 0x00, \
> + 0x80, 0x00, 0x00, 0xaa, 0x00, 0x38, 0x9b, 0x71}
> +#define UVC_GUID_FORMAT_Y10 \
> + { 'Y', '1', '0', ' ', 0x00, 0x00, 0x10, 0x00, \
> + 0x80, 0x00, 0x00, 0xaa, 0x00, 0x38, 0x9b, 0x71}
> +#define UVC_GUID_FORMAT_Y12 \
> + { 'Y', '1', '2', ' ', 0x00, 0x00, 0x10, 0x00, \
> + 0x80, 0x00, 0x00, 0xaa, 0x00, 0x38, 0x9b, 0x71}
> #define UVC_GUID_FORMAT_Y16 \
> { 'Y', '1', '6', ' ', 0x00, 0x00, 0x10, 0x00, \
>0x80, 0x00, 0x00, 0xaa, 0x00, 0x38, 0x9b, 0x71}
--
Regards,
Laurent Pinchart
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 5/5] drivers/media/platform/omap3isp/isp.c: fix error return code
Hi,
On Sunday 23 September 2012 19:46:53 Peter Senna Tschudin wrote:
> On Sun, Sep 23, 2012 at 7:39 PM, Mauro Carvalho Chehab wrote:
> > Laurent,
> >
> > Could you please review this patch?
> >
> > Peter,
> >
> > Please, always c/c the driver maintainer/author on patches you submit.
> >
> > You can check it with scripts/get_maintainer.pl.
>
> Sorry for that. I'll be more careful next time. Thanks!
>
> > Regards,
> > Mauro
> >
> > Mensagem original
> > Assunto: [PATCH 5/5] drivers/media/platform/omap3isp/isp.c: fix error
> > return code Data: Tue, 4 Sep 2012 18:14:25 +0200
> > De: Peter Senna Tschudin
> > Para: Mauro Carvalho Chehab
> > CC: kernel-janit...@vger.kernel.org, julia.law...@lip6.fr,
> > linux-me...@vger.kernel.org, linux-kernel@vger.kernel.org
> >
> > From: Peter Senna Tschudin
> >
> > Convert a nonnegative error return code to a negative one, as returned
> > elsewhere in the function.
> >
> > A simplified version of the semantic match that finds this problem is as
> > follows: (http://coccinelle.lip6.fr/)
> >
> > //
> > (
> > if@p1 (\(ret < 0\|ret != 0\))
> >
> > { ... return ret; }
> >
> > ret@p1 = 0
> > )
> > ... when != ret = e1
> >
> > when != &ret
> >
> > *if(...)
> > {
> >
> > ... when != ret = e2
> >
> > when forall
> >
> > return ret;
> >
> > }
> >
> > //
> >
> > Signed-off-by: Peter Senna Tschudin
> >
> > ---
> >
> > drivers/media/platform/omap3isp/isp.c |4 +++-
> > 1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/media/platform/omap3isp/isp.c
> > b/drivers/media/platform/omap3isp/isp.c index e0096e0..91fcaef 100644
> > --- a/drivers/media/platform/omap3isp/isp.c
> > +++ b/drivers/media/platform/omap3isp/isp.c
> > @@ -2102,8 +2102,10 @@ static int __devinit isp_probe(struct
> > platform_device *pdev)>
> > if (ret < 0)
> > goto error;
> >
> > - if (__omap3isp_get(isp, false) == NULL)
> > + if (__omap3isp_get(isp, false) == NULL) {
> > + ret = -EBUSY; /* Not sure if EBUSY is best for here */
I've replaced -EBUSY with -ENODEV, removed the comment and applied the patch
to my tree. Thanks.
> > goto error;
> > + }
> >
> > ret = isp_reset(isp);
> > if (ret < 0)
--
Regards,
Laurent Pinchart
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v5] media: v4l2-ctrl: add a helper function to add standard control with driver specific menu
On Monday 24 September 2012 18:23:40 Prabhakar wrote: > From: Lad, Prabhakar > > Add helper function v4l2_ctrl_new_std_menu_items(), which adds > a standard menu control, with driver specific menu. > > Signed-off-by: Lad, Prabhakar > Signed-off-by: Manjunath Hadli > Cc: Hans Verkuil > Cc: Sakari Ailus > Cc: Sylwester Nawrocki > Cc: Laurent Pinchart > Cc: Mauro Carvalho Chehab > Cc: Hans de Goede > Cc: Kyungmin Park > Cc: Guennadi Liakhovetski > Cc: Rob Landley Thank you for not giving up despite the many change requests you have received :-) Acked-by: Laurent Pinchart -- Regards, Laurent Pinchart -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] media: davinci: vpif: set device capabilities
Hi Hans,
On Tuesday 25 September 2012 13:43:36 Hans Verkuil wrote:
> On Tue 25 September 2012 13:16:24 Prabhakar wrote:
> > From: Lad, Prabhakar
> >
> > Signed-off-by: Lad, Prabhakar
> > Signed-off-by: Manjunath Hadli
> > Cc: Hans Verkuil
> > ---
> >
> > drivers/media/platform/davinci/vpif_capture.c |4 +++-
> > drivers/media/platform/davinci/vpif_display.c |4 +++-
> > 2 files changed, 6 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/media/platform/davinci/vpif_capture.c
> > b/drivers/media/platform/davinci/vpif_capture.c index 482..faeca98
> > 100644
> > --- a/drivers/media/platform/davinci/vpif_capture.c
> > +++ b/drivers/media/platform/davinci/vpif_capture.c
> > @@ -1630,7 +1630,9 @@ static int vpif_querycap(struct file *file, void
> > *priv,>
> > {
> >
> > struct vpif_capture_config *config = vpif_dev->platform_data;
> >
> > - cap->capabilities = V4L2_CAP_VIDEO_CAPTURE | V4L2_CAP_STREAMING;
> > + cap->device_caps = V4L2_CAP_VIDEO_CAPTURE | V4L2_CAP_STREAMING |
> > + V4L2_CAP_READWRITE;
> > + cap->capabilities = cap->device_caps | V4L2_CAP_DEVICE_CAPS;
> >
> > strlcpy(cap->driver, "vpif capture", sizeof(cap->driver));
>
> This should be the real driver name which is 'vpif_capture'.
>
> > strlcpy(cap->bus_info, "VPIF Platform", sizeof(cap->bus_info));
>
> For bus_info I would use: "platform:vpif_capture".
>
> The 'platform:' prefix is going to be the standard for platform drivers.
What about
snprintf(cap->driver, sizeof(cap->driver), "platform:%s", dev_name(vpif_dev));
That would handle cases where multiple platform devices of the same type are
present.
> > strlcpy(cap->card, config->card_name, sizeof(cap->card));
> >
> > diff --git a/drivers/media/platform/davinci/vpif_display.c
> > b/drivers/media/platform/davinci/vpif_display.c index d94b8a2..171e449
> > 100644
> > --- a/drivers/media/platform/davinci/vpif_display.c
> > +++ b/drivers/media/platform/davinci/vpif_display.c
> > @@ -827,7 +827,9 @@ static int vpif_querycap(struct file *file, void
> > *priv,>
> > {
> >
> > struct vpif_display_config *config = vpif_dev->platform_data;
> >
> > - cap->capabilities = V4L2_CAP_VIDEO_OUTPUT | V4L2_CAP_STREAMING;
> > + cap->device_caps = V4L2_CAP_VIDEO_OUTPUT | V4L2_CAP_STREAMING |
> > + V4L2_CAP_READWRITE;
> > + cap->capabilities = cap->device_caps | V4L2_CAP_DEVICE_CAPS;
> >
> > strlcpy(cap->driver, "vpif display", sizeof(cap->driver));
>
> vpif_driver
>
> > strlcpy(cap->bus_info, "Platform", sizeof(cap->bus_info));
>
> Ditto: "platform:vpif_driver".
>
> > strlcpy(cap->card, config->card_name, sizeof(cap->card));
--
Regards,
Laurent Pinchart
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] v4l: change path of video drivers
Hi Prabhakar, Thanks for the patch. On Friday 14 September 2012 14:47:52 Prabhakar Lad wrote: > From: Lad, Prabhakar > > due to structure change for video drivers, change the > description with correct path. > > Signed-off-by: Lad, Prabhakar > Signed-off-by: Manjunath Hadli > Cc: Mauro Carvalho Chehab > Cc: Hans Verkuil > Cc: Laurent Pinchart > Cc: Sakari Ailus > Cc: Rob Landley For the omap3isp part and the V4L2 core, Acked-by: Laurent Pinchart > --- > Documentation/video4linux/CQcam.txt |2 +- > Documentation/video4linux/README.davinci-vpbe | 20 ++-- > Documentation/video4linux/fimc.txt| 16 > Documentation/video4linux/omap3isp.txt|2 +- > Documentation/video4linux/v4l2-framework.txt |2 +- > Documentation/video4linux/videobuf|2 +- > 6 files changed, 22 insertions(+), 22 deletions(-) > > diff --git a/Documentation/video4linux/CQcam.txt > b/Documentation/video4linux/CQcam.txt index 6e680fe..0b69e4e 100644 > --- a/Documentation/video4linux/CQcam.txt > +++ b/Documentation/video4linux/CQcam.txt > @@ -18,7 +18,7 @@ Table of Contents > > 1.0 Introduction > > - The file ../../drivers/media/video/c-qcam.c is a device driver for > + The file ../../drivers/media/parport/c-qcam.c is a device driver for > the Logitech (nee Connectix) parallel port interface color CCD camera. > This is a fairly inexpensive device for capturing images. Logitech > does not currently provide information for developers, but many people > diff --git a/Documentation/video4linux/README.davinci-vpbe > b/Documentation/video4linux/README.davinci-vpbe index 7a460b0..dc9a297 > 100644 > --- a/Documentation/video4linux/README.davinci-vpbe > +++ b/Documentation/video4linux/README.davinci-vpbe > @@ -5,22 +5,22 @@ > File partitioning > - > V4L2 display device driver > - drivers/media/video/davinci/vpbe_display.c > - drivers/media/video/davinci/vpbe_display.h > + drivers/media/platform/davinci/vpbe_display.c > + drivers/media/platform/davinci/vpbe_display.h > > VPBE display controller > - drivers/media/video/davinci/vpbe.c > - drivers/media/video/davinci/vpbe.h > + drivers/media/platform/davinci/vpbe.c > + drivers/media/platform/davinci/vpbe.h > > VPBE venc sub device driver > - drivers/media/video/davinci/vpbe_venc.c > - drivers/media/video/davinci/vpbe_venc.h > - drivers/media/video/davinci/vpbe_venc_regs.h > + drivers/media/platform/davinci/vpbe_venc.c > + drivers/media/platform/davinci/vpbe_venc.h > + drivers/media/platform/davinci/vpbe_venc_regs.h > > VPBE osd driver > - drivers/media/video/davinci/vpbe_osd.c > - drivers/media/video/davinci/vpbe_osd.h > - drivers/media/video/davinci/vpbe_osd_regs.h > + drivers/media/platform/davinci/vpbe_osd.c > + drivers/media/platform/davinci/vpbe_osd.h > + drivers/media/platform/davinci/vpbe_osd_regs.h > > Functional partitioning > --- > diff --git a/Documentation/video4linux/fimc.txt > b/Documentation/video4linux/fimc.txt index eb04970..fd02d9a 100644 > --- a/Documentation/video4linux/fimc.txt > +++ b/Documentation/video4linux/fimc.txt > @@ -10,7 +10,7 @@ data from LCD controller (FIMD) through the SoC internal > writeback data path. There are multiple FIMC instances in the SoCs (up to > 4), having slightly different capabilities, like pixel alignment > constraints, rotator availability, LCD writeback support, etc. The driver > is located at -drivers/media/video/s5p-fimc directory. > +drivers/media/platform/s5p-fimc directory. > > 1. Supported SoCs > = > @@ -36,21 +36,21 @@ Not currently supported: > = > > - media device driver > - drivers/media/video/s5p-fimc/fimc-mdevice.[ch] > + drivers/media/platform/s5p-fimc/fimc-mdevice.[ch] > > - camera capture video device driver > - drivers/media/video/s5p-fimc/fimc-capture.c > + drivers/media/platform/s5p-fimc/fimc-capture.c > > - MIPI-CSI2 receiver subdev > - drivers/media/video/s5p-fimc/mipi-csis.[ch] > + drivers/media/platform/s5p-fimc/mipi-csis.[ch] > > - video post-processor (mem-to-mem) > - drivers/media/video/s5p-fimc/fimc-core.c > + drivers/media/platform/s5p-fimc/fimc-core.c > > - common files > - drivers/media/video/s5p-fimc/fimc-core.h > - drivers/media/video/s5p-fimc/fimc-reg.h > - drivers/media/video/s5p-fimc/regs-fimc.h > + drivers/media/platform/s5p-fimc/fimc-core.h > + drivers/media/platform/s5p-fimc/fimc-reg.h > + d
Re: [PATCH] media: davinci: vpif: set device capabilities
On Tuesday 25 September 2012 15:48:10 Hans Verkuil wrote:
> On Tue 25 September 2012 15:26:11 Prabhakar Lad wrote:
> > On Tue, Sep 25, 2012 at 5:24 PM, Hans Verkuil wrote:
> > > On Tue 25 September 2012 13:49:16 Laurent Pinchart wrote:
> > >> Hi Hans,
> > >>
> > >> On Tuesday 25 September 2012 13:43:36 Hans Verkuil wrote:
> > >> > On Tue 25 September 2012 13:16:24 Prabhakar wrote:
> > >> > > From: Lad, Prabhakar
> > >> > >
> > >> > > Signed-off-by: Lad, Prabhakar
> > >> > > Signed-off-by: Manjunath Hadli
> > >> > > Cc: Hans Verkuil
> > >> > > ---
> > >> > >
> > >> > > drivers/media/platform/davinci/vpif_capture.c |4 +++-
> > >> > > drivers/media/platform/davinci/vpif_display.c |4 +++-
> > >> > > 2 files changed, 6 insertions(+), 2 deletions(-)
> > >> > >
> > >> > > diff --git a/drivers/media/platform/davinci/vpif_capture.c
> > >> > > b/drivers/media/platform/davinci/vpif_capture.c index
> > >> > > 482..faeca98
> > >> > > 100644
> > >> > > --- a/drivers/media/platform/davinci/vpif_capture.c
> > >> > > +++ b/drivers/media/platform/davinci/vpif_capture.c
> > >> > > @@ -1630,7 +1630,9 @@ static int vpif_querycap(struct file *file,
> > >> > > void
> > >> > > *priv,>
> > >> > >
> > >> > > {
> > >> > >
> > >> > > struct vpif_capture_config *config = vpif_dev->platform_data;
> > >> > >
> > >> > > - cap->capabilities = V4L2_CAP_VIDEO_CAPTURE | V4L2_CAP_STREAMING;
> > >> > > + cap->device_caps = V4L2_CAP_VIDEO_CAPTURE | V4L2_CAP_STREAMING |
> > >> > > + V4L2_CAP_READWRITE;
> > >> > > + cap->capabilities = cap->device_caps | V4L2_CAP_DEVICE_CAPS;
> > >> > >
> > >> > > strlcpy(cap->driver, "vpif capture", sizeof(cap->driver));
> > >> >
> > >> > This should be the real driver name which is 'vpif_capture'.
> > >> >
> > >> > > strlcpy(cap->bus_info, "VPIF Platform", sizeof(cap->bus_info));
> > >> >
> > >> > For bus_info I would use: "platform:vpif_capture".
> > >> >
> > >> > The 'platform:' prefix is going to be the standard for platform
> > >> > drivers.
> > >>
> > >> What about
> > >>
> > >> snprintf(cap->driver, sizeof(cap->driver), "platform:%s",
> > >> dev_name(vpif_dev));
> > >>
> > >> That would handle cases where multiple platform devices of the same
> > >> type are present.
> > >
> > > Sure, that's even better. You do have to check that this gives you what
> > > you'd expect (i.e., that you don't end up with
> > > "platform:platform:vpif_capture").>
> > But the driver field is max 16, should i extend it to 32 ?
>
> I'm certain Laurent meant to say:
>
> snprintf(cap->bus_info, sizeof(cap->bus_info), "platform:%s",
> dev_name(vpif_dev));
Yes that's what I meant, sorry.
> It makes no sense to use cap->driver.
--
Regards,
Laurent Pinchart
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] media: mt9p031/mt9t001/mt9v032: use V4L2_CID_TEST_PATTERN for test pattern control
= freq;
> mt9v032->sysclk = freq;
> break;
> -
> case V4L2_CID_TEST_PATTERN:
> + mt9v032->test_pattern = ctrl->val;
> + if (mt9v032->test_pattern > 3)
> + break;
No need to store this, just select the requested test pattern.
> switch (ctrl->val) {
> case 0:
> data = 0;
> @@ -561,15 +564,18 @@ static int mt9v032_s_ctrl(struct v4l2_ctrl *ctrl)
> data = MT9V032_TEST_PATTERN_GRAY_DIAGONAL
>
>| MT9V032_TEST_PATTERN_ENABLE;
>
> break;
> - default:
> - data = (ctrl->val << MT9V032_TEST_PATTERN_DATA_SHIFT)
> - | MT9V032_TEST_PATTERN_USE_DATA
> - | MT9V032_TEST_PATTERN_ENABLE
> - | MT9V032_TEST_PATTERN_FLIP;
> - break;
This will need to be kept.
> }
>
> return mt9v032_write(client, MT9V032_TEST_PATTERN, data);
> +
> + case V4L2_CID_PARAMETRIC_TEST_PATTERN:
> + if (mt9v032->test_pattern != 4)
> + return 0;
No need to check that, just write the test pattern color to the
MT9V032_TEST_PATTERN_DATA field without changing the rest of the register. You
could also put the two test pattern controls in a cluster.
> + data = (ctrl->val << MT9V032_TEST_PATTERN_DATA_SHIFT)
> + | MT9V032_TEST_PATTERN_USE_DATA
> + | MT9V032_TEST_PATTERN_ENABLE
> + | MT9V032_TEST_PATTERN_FLIP;
> + return mt9v032_write(client, MT9V032_TEST_PATTERN, data);
> }
>
> return 0;
> @@ -579,16 +585,24 @@ static struct v4l2_ctrl_ops mt9v032_ctrl_ops = {
> .s_ctrl = mt9v032_s_ctrl,
> };
>
> +static const char * const mt9v032_test_pattern_menu[] = {
> + "Disabled",
> + "Gray Vertical Shade",
> + "Gray Horizontal Shade",
> + "Gray Diagonal Shade",
> + "Parametric Test Pattern",
I would just say "Plain".
> +};
> +
> static const struct v4l2_ctrl_config mt9v032_ctrls[] = {
> {
> .ops= &mt9v032_ctrl_ops,
> - .id = V4L2_CID_TEST_PATTERN,
> + .id = V4L2_CID_PARAMETRIC_TEST_PATTERN,
> .type = V4L2_CTRL_TYPE_INTEGER,
> - .name = "Test pattern",
> - .min= 0,
> + .name = "Parametric Test Pattern Values",
"Test Pattern Color"
> + .min= 4,
> .max= 1023,
> .step = 1,
> - .def = 0,
> + .def= 4,
> .flags = 0,
> }
> };
> @@ -741,7 +755,7 @@ static int mt9v032_probe(struct i2c_client *client,
> mutex_init(&mt9v032->power_lock);
> mt9v032->pdata = pdata;
>
> - v4l2_ctrl_handler_init(&mt9v032->ctrls, ARRAY_SIZE(mt9v032_ctrls) + 8);
> + v4l2_ctrl_handler_init(&mt9v032->ctrls, ARRAY_SIZE(mt9v032_ctrls) + 9);
>
> v4l2_ctrl_new_std(&mt9v032->ctrls, &mt9v032_ctrl_ops,
> V4L2_CID_AUTOGAIN, 0, 1, 1, 1);
> @@ -763,6 +777,10 @@ static int mt9v032_probe(struct i2c_client *client,
> V4L2_CID_VBLANK, MT9V032_VERTICAL_BLANKING_MIN,
> MT9V032_VERTICAL_BLANKING_MAX, 1,
> MT9V032_VERTICAL_BLANKING_DEF);
> + v4l2_ctrl_new_std_menu_items(&mt9v032->ctrls, &mt9v032_ctrl_ops,
> + V4L2_CID_TEST_PATTERN,
> + ARRAY_SIZE(mt9v032_test_pattern_menu) - 1, 0,
> + 0, mt9v032_test_pattern_menu);
>
> mt9v032->pixel_rate =
> v4l2_ctrl_new_std(&mt9v032->ctrls, &mt9v032_ctrl_ops,
--
Regards,
Laurent Pinchart
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2] media: mt9p031/mt9t001/mt9v032: use V4L2_CID_TEST_PATTERN for test pattern control
Hi Prabhakar,
Thanks for the patch.
On Wednesday 26 September 2012 12:05:10 Prabhakar wrote:
> From: Lad, Prabhakar
>
> Signed-off-by: Lad, Prabhakar
> Signed-off-by: Manjunath Hadli
> Cc: Laurent Pinchart
> Cc: Sakari Ailus
> Cc: Paul Gortmaker
> Cc: Jean Delvare
> ---
> Changes for v2:
> 1: Fixed review comments pointed by Laurent.
>
> drivers/media/i2c/mt9p031.c | 19 +--
> drivers/media/i2c/mt9t001.c | 23 +++
> drivers/media/i2c/mt9v032.c | 34 --
> 3 files changed, 44 insertions(+), 32 deletions(-)
>
> diff --git a/drivers/media/i2c/mt9p031.c b/drivers/media/i2c/mt9p031.c
> index 2c0f407..e328332 100644
> --- a/drivers/media/i2c/mt9p031.c
> +++ b/drivers/media/i2c/mt9p031.c
> @@ -574,7 +574,6 @@ static int mt9p031_set_crop(struct v4l2_subdev *subdev,
> * V4L2 subdev control operations
> */
>
> -#define V4L2_CID_TEST_PATTERN(V4L2_CID_USER_BASE | 0x1001)
> #define V4L2_CID_BLC_AUTO(V4L2_CID_USER_BASE | 0x1002)
> #define V4L2_CID_BLC_TARGET_LEVEL(V4L2_CID_USER_BASE | 0x1003)
> #define V4L2_CID_BLC_ANALOG_OFFSET (V4L2_CID_USER_BASE | 0x1004)
> @@ -740,18 +739,6 @@ static const char * const mt9p031_test_pattern_menu[] =
> { static const struct v4l2_ctrl_config mt9p031_ctrls[] = {
> {
> .ops= &mt9p031_ctrl_ops,
> - .id = V4L2_CID_TEST_PATTERN,
> - .type = V4L2_CTRL_TYPE_MENU,
> - .name = "Test Pattern",
> - .min= 0,
> - .max= ARRAY_SIZE(mt9p031_test_pattern_menu) - 1,
> - .step = 0,
> - .def= 0,
> - .flags = 0,
> - .menu_skip_mask = 0,
> - .qmenu = mt9p031_test_pattern_menu,
> - }, {
> - .ops= &mt9p031_ctrl_ops,
> .id = V4L2_CID_BLC_AUTO,
> .type = V4L2_CTRL_TYPE_BOOLEAN,
> .name = "BLC, Auto",
> @@ -950,7 +937,7 @@ static int mt9p031_probe(struct i2c_client *client,
> mt9p031->model = did->driver_data;
> mt9p031->reset = -1;
>
> - v4l2_ctrl_handler_init(&mt9p031->ctrls, ARRAY_SIZE(mt9p031_ctrls) + 5);
> + v4l2_ctrl_handler_init(&mt9p031->ctrls, ARRAY_SIZE(mt9p031_ctrls) + 6);
>
> v4l2_ctrl_new_std(&mt9p031->ctrls, &mt9p031_ctrl_ops,
> V4L2_CID_EXPOSURE, MT9P031_SHUTTER_WIDTH_MIN,
> @@ -966,6 +953,10 @@ static int mt9p031_probe(struct i2c_client *client,
> v4l2_ctrl_new_std(&mt9p031->ctrls, &mt9p031_ctrl_ops,
> V4L2_CID_PIXEL_RATE, pdata->target_freq,
> pdata->target_freq, 1, pdata->target_freq);
> + v4l2_ctrl_new_std_menu_items(&mt9p031->ctrls, &mt9p031_ctrl_ops,
> + V4L2_CID_TEST_PATTERN,
> + ARRAY_SIZE(mt9p031_test_pattern_menu) - 1, 0,
> + 0, mt9p031_test_pattern_menu);
>
> for (i = 0; i < ARRAY_SIZE(mt9p031_ctrls); ++i)
> v4l2_ctrl_new_custom(&mt9p031->ctrls, &mt9p031_ctrls[i], NULL);
> diff --git a/drivers/media/i2c/mt9t001.c b/drivers/media/i2c/mt9t001.c
> index 6d343ad..23f4e0a 100644
> --- a/drivers/media/i2c/mt9t001.c
> +++ b/drivers/media/i2c/mt9t001.c
> @@ -371,7 +371,7 @@ static int mt9t001_set_crop(struct v4l2_subdev *subdev,
> * V4L2 subdev control operations
> */
>
> -#define V4L2_CID_TEST_PATTERN(V4L2_CID_USER_BASE | 0x1001)
> +#define V4L2_CID_TEST_PATTERN_COLOR (V4L2_CID_USER_BASE | 0x1001)
> #define V4L2_CID_BLACK_LEVEL_AUTO(V4L2_CID_USER_BASE | 0x1002)
> #define V4L2_CID_BLACK_LEVEL_OFFSET (V4L2_CID_USER_BASE | 0x1003)
> #define V4L2_CID_BLACK_LEVEL_CALIBRATE (V4L2_CID_USER_BASE | 0x1004)
> @@ -485,14 +485,12 @@ static int mt9t001_s_ctrl(struct v4l2_ctrl *ctrl)
>
> return mt9t001_write(client, MT9T001_SHUTTER_WIDTH_HIGH,
>ctrl->val >> 16);
> -
No need to remove the blank line here.
> case V4L2_CID_TEST_PATTERN:
> - ret = mt9t001_set_output_control(mt9t001,
> + return mt9t001_set_output_control(mt9t001,
> ctrl->val ? 0 : MT9T001_OUTPUT_CONTROL_TEST_DATA,
> ctrl->val ? MT9T001_OUTPUT_CONTROL_TEST_DATA : 0);
> - if (ret < 0)
> - return ret;
>
> + case V4L2_CID_TEST_PATTERN_COLOR:
> return mt9t
Re: [PATCH] drm: platform: Don't initialize driver-private data
Hi Thierry,
Thank you for the patch, and sorry for the late reply.
On Monday 15 October 2012 20:03:42 Thierry Reding wrote:
> Platform device drivers usually use the driver-private data for their
> own purposes. Having it overwritten by drm_platform_init() is confusing
> and error-prone.
If you want to push drivers that way, you should get rid of the
pci_set_drvdata() call in core DRM as well. This would push device driver data
handling down to all drivers, so I'm not convinced it would actually make
things simpler.
> Signed-off-by: Thierry Reding
Tested-by: Laurent Pinchart
But I'm not convinced by the patch, as explained above.
> ---
> Note that I don't have any hardware to test the shmobile changes on so
> it would be good to get a Tested-by for that.
>
> drivers/gpu/drm/drm_platform.c | 1 -
> drivers/gpu/drm/shmobile/shmob_drm_drv.c | 12 +---
> 2 files changed, 5 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_platform.c b/drivers/gpu/drm/drm_platform.c
> index aaeb6f8..b8a282e 100644
> --- a/drivers/gpu/drm/drm_platform.c
> +++ b/drivers/gpu/drm/drm_platform.c
> @@ -64,7 +64,6 @@ int drm_get_platform_dev(struct platform_device *platdev,
> }
>
> if (drm_core_check_feature(dev, DRIVER_MODESET)) {
> - dev_set_drvdata(&platdev->dev, dev);
> ret = drm_get_minor(dev, &dev->control, DRM_MINOR_CONTROL);
> if (ret)
> goto err_g1;
> diff --git a/drivers/gpu/drm/shmobile/shmob_drm_drv.c
> b/drivers/gpu/drm/shmobile/shmob_drm_drv.c index c71d493..1c350fc 100644
> --- a/drivers/gpu/drm/shmobile/shmob_drm_drv.c
> +++ b/drivers/gpu/drm/shmobile/shmob_drm_drv.c
> @@ -201,6 +201,8 @@ static int shmob_drm_load(struct drm_device *dev,
> unsigned long flags) goto done;
> }
>
> + platform_set_drvdata(pdev, sdev);
> +
> done:
> if (ret)
> shmob_drm_unload(dev);
> @@ -299,11 +301,9 @@ static struct drm_driver shmob_drm_driver = {
> #if CONFIG_PM_SLEEP
> static int shmob_drm_pm_suspend(struct device *dev)
> {
> - struct platform_device *pdev = to_platform_device(dev);
> - struct drm_device *ddev = platform_get_drvdata(pdev);
> - struct shmob_drm_device *sdev = ddev->dev_private;
> + struct shmob_drm_device *sdev = dev_get_drvdata(dev);
>
> - drm_kms_helper_poll_disable(ddev);
> + drm_kms_helper_poll_disable(sdev->ddev);
> shmob_drm_crtc_suspend(&sdev->crtc);
>
> return 0;
> @@ -311,9 +311,7 @@ static int shmob_drm_pm_suspend(struct device *dev)
>
> static int shmob_drm_pm_resume(struct device *dev)
> {
> - struct platform_device *pdev = to_platform_device(dev);
> - struct drm_device *ddev = platform_get_drvdata(pdev);
> - struct shmob_drm_device *sdev = ddev->dev_private;
> + struct shmob_drm_device *sdev = dev_get_drvdata(dev);
>
> mutex_lock(&sdev->ddev->mode_config.mutex);
> shmob_drm_crtc_resume(&sdev->crtc);
--
Regards,
Laurent Pinchart
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] drm: platform: Don't initialize driver-private data
Hi Thierry, On Wednesday 31 October 2012 09:26:07 Thierry Reding wrote: > On Fri, Oct 26, 2012 at 04:06:27PM +0200, Laurent Pinchart wrote: > > On Monday 15 October 2012 20:03:42 Thierry Reding wrote: > > > Platform device drivers usually use the driver-private data for their > > > own purposes. Having it overwritten by drm_platform_init() is confusing > > > and error-prone. > > > > If you want to push drivers that way, you should get rid of the > > pci_set_drvdata() call in core DRM as well. This would push device driver > > data handling down to all drivers, so I'm not convinced it would actually > > make things simpler. > > I think the problem doesn't exist for PCI-based DRM drivers, so I didn't > look at it. The issue only arises once the DRM needs to glue together > multiple devices, as is usual with the drivers for embedded devices, > where the drivers are based on platform devices. > > I agree, though, that for consistency it would be nicer not to do this > for the PCI-based DRM drivers either. If David agrees I can take a look > at converting the other drivers along with the change to the DRM core. > > Pushing the handling of the driver-private data down to the drivers may > not make things easier, but at least it would be consistent with other > drivers. I didn't mention this in the patch description but it actually > took me a day to track down why the driver kept crashing until I figured > out that drm_platform_init() actually modified the pointer. So we either need your patch, or a documentation update :-) The patch itself is fine, I'll let others comment on the approach. -- Regards, Laurent Pinchart signature.asc Description: This is a digitally signed message part.
Re: [PATCH v2 3/3] gpio: pcf857x: Add OF support
Hi Tomasz,
On Saturday 24 August 2013 16:13:11 Tomasz Figa wrote:
> On Saturday 24 of August 2013 02:54:07 Laurent Pinchart wrote:
> > On Saturday 24 August 2013 02:41:59 Tomasz Figa wrote:
> > > On Tuesday 20 of August 2013 01:04:54 Laurent Pinchart wrote:
> > > > Add DT bindings for the pcf857x-compatible chips and parse the
> > > > device tree node in the driver.
> > > >
> > > > Signed-off-by: Laurent Pinchart
> > > > ---
> > > >
> > > > .../devicetree/bindings/gpio/gpio-pcf857x.txt | 71 +
> > > > drivers/gpio/gpio-pcf857x.c| 57 ++---
> > > > 2 files changed, 119 insertions(+), 9 deletions(-)
> > > > create mode 100644
[snip]
> > > > diff --git a/drivers/gpio/gpio-pcf857x.c
> > > > b/drivers/gpio/gpio-pcf857x.c
> > > > index 070e81f..50a90f1 100644
> > > > --- a/drivers/gpio/gpio-pcf857x.c
> > > > +++ b/drivers/gpio/gpio-pcf857x.c
> >
> > [snip]
> >
> > > > @@ -50,6 +52,27 @@ static const struct i2c_device_id pcf857x_id[] =
> > > > {
> > > >
> > > > };
> > > > MODULE_DEVICE_TABLE(i2c, pcf857x_id);
> > > >
> > > > +#ifdef CONFIG_OF
> > > > +static const struct of_device_id pcf857x_of_table[] = {
> > > > + { .compatible = "nxp,pcf8574", .data = (void *)8 },
> > > > + { .compatible = "nxp,pcf8574a", .data = (void *)8 },
> > > > + { .compatible = "nxp,pca8574", .data = (void *)8 },
> > > > + { .compatible = "nxp,pca9670", .data = (void *)8 },
> > > > + { .compatible = "nxp,pca9672", .data = (void *)8 },
> > > > + { .compatible = "nxp,pca9674", .data = (void *)8 },
> > > > + { .compatible = "nxp,pcf8575", .data = (void *)16 },
> > > > + { .compatible = "nxp,pca8575", .data = (void *)16 },
> > > > + { .compatible = "nxp,pca9671", .data = (void *)16 },
> > > > + { .compatible = "nxp,pca9673", .data = (void *)16 },
> > > > + { .compatible = "nxp,pca9675", .data = (void *)16 },
> > > > + { .compatible = "maxim,max7328", .data = (void *)8 },
> > > > + { .compatible = "maxim,max7329", .data = (void *)8 },
> > > > + { .compatible = "ti,tca9554", .data = (void *)8 },
> > > > + { }
> > > > +};
> > > > +MODULE_DEVICE_TABLE(of, pcf857x_of_table);
> > > > +#endif
> > > > +
> > > >
> > > > /*
> > > > * The pcf857x, pca857x, and pca967x chips only expose one read and
> > > > * one write register. Writing a "one" bit (to match the reset
> > > > @@ -257,14 +280,29 @@ fail:
> > > > static int pcf857x_probe(struct i2c_client *client,
> > > > const struct i2c_device_id *id)
> > > > {
> > > > - struct pcf857x_platform_data*pdata;
> > > > + struct pcf857x_platform_data*pdata =
> > > > client->dev.platform_data;
> > > > + struct device_node *np = client->dev.of_node;
> > > struct pcf857x *gpio;
> > > > + unsigned intn_latch = 0;
> > > > + unsigned intngpio;
> > > > int status;
> > > >
> > > > - pdata = client->dev.platform_data;
> > > > - if (!pdata) {
> > > > +#ifdef CONFIG_OF
> > > > + if (np) {
> > >
> > > Wouldn't if (IS_ENABLED(CONFIG_OF) && np) be sufficient here, without
> > > the #ifdef? You would have to move the match table out of the #ifdef
> > > in this case, though...
> >
> > That's the exact reason why I've used #ifdef CONFIG_OF here, I didn't
> > want to add the overhead of the pcf857x_of_table when CONFIG_OF isn't
> > defined.
>
> I'm not sure if I remember correctly, but I think there was something said
> in one of discussions some time ago, that we should be moving away from
> ifdef'ing such things, in favour of just having them compiled
> unconditionally.
There seems to be a general consensus to favor if (IS_ENABLED(CONFIG_OF))
instead of #ifdef CONFIG_OF when possible. I'm not sure what the opinion is
regarding using conditional compilation to avoid compiling unnecessary data
tables in. I would vote for using it (there's no need to bloat the kernel
unnecessarily on non-OF platforms), but I'll conform to whatever is decided to
be best.
> [Adding DT maintainers on Cc for more opinions.]
I'll resubmit the patch with the DT bindings documentation fixed, and will
submit yet another version if I need to remove the #ifdef.
--
Regards,
Laurent Pinchart
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH v3] gpio: pcf857x: Add OF support
Add DT bindings for the pcf857x-compatible chips and parse the device
tree node in the driver.
Signed-off-by: Laurent Pinchart
---
.../devicetree/bindings/gpio/gpio-pcf857x.txt | 71 ++
drivers/gpio/gpio-pcf857x.c| 57 ++---
2 files changed, 119 insertions(+), 9 deletions(-)
create mode 100644 Documentation/devicetree/bindings/gpio/gpio-pcf857x.txt
Changes since v2
- Replace mention about interrupts software configuration in DT bindings
documentation with an explanation of the hardware configuration.
diff --git a/Documentation/devicetree/bindings/gpio/gpio-pcf857x.txt
b/Documentation/devicetree/bindings/gpio/gpio-pcf857x.txt
new file mode 100644
index 000..d0a3af4
--- /dev/null
+++ b/Documentation/devicetree/bindings/gpio/gpio-pcf857x.txt
@@ -0,0 +1,71 @@
+* PCF857x-compatible I/O expanders
+
+The PCF857x-compatible chips have "quasi-bidirectional" I/O pins that can be
+driven high by a pull-up current source or driven low to ground. This combines
+the direction and output level into a single bit per pin, which can't be read
+back. We can't actually know at initialization time whether a pin is configured
+(a) as output and driving the signal low/high, or (b) as input and reporting a
+low/high value, without knowing the last value written since the chip came out
+of reset (if any). The only reliable solution for setting up pin direction is
+thus to do it explicitly.
+
+Required Properties:
+
+ - compatible: should be one of the following.
+- "maxim,max7328": For the Maxim MAX7378
+- "maxim,max7329": For the Maxim MAX7329
+- "nxp,pca8574": For the NXP PCA8574
+- "nxp,pca8575": For the NXP PCA8575
+- "nxp,pca9670": For the NXP PCA9670
+- "nxp,pca9671": For the NXP PCA9671
+- "nxp,pca9672": For the NXP PCA9672
+- "nxp,pca9673": For the NXP PCA9673
+- "nxp,pca9674": For the NXP PCA9674
+- "nxp,pca9675": For the NXP PCA9675
+- "nxp,pcf8574": For the NXP PCF8574
+- "nxp,pcf8574a": For the NXP PCF8574A
+- "nxp,pcf8575": For the NXP PCF8575
+- "ti,tca9554": For the TI TCA9554
+
+ - reg: I2C slave address.
+
+ - gpio-controller: Marks the device node as a gpio controller.
+ - #gpio-cells: Should be 2. The first cell is the GPIO number and the second
+cell specifies GPIO flags, as defined in . Only
the
+GPIO_ACTIVE_HIGH and GPIO_ACTIVE_LOW flags are supported.
+
+Optional Properties:
+
+ - pins-initial-state: Bitmask that specifies the initial state of each pin.
+ When a bit is set to zero, the corresponding pin will be initialized to the
+ input (pulled-up) state. When the bit is set to one, the pin will be
+ initialized the the low-level output state. If the property is not specified
+ all pins will be initialized to the input state.
+
+ The I/O expander can detect input state changes, and thus optionally act as
+ an interrupt controller. When the expander interrupt pin is connected all the
+ following properties must be set. For more information please see the
+ interrupt controller device tree bindings documentation available at
+ Documentation/devicetree/bindings/interrupt-controller/interrupts.txt.
+
+ - interrupt-controller: Identifies the node as an interrupt controller.
+ - #interrupt-cells: Number of cells to encode an interrupt source, shall be
2.
+ - interrupt-parent: phandle of the parent interrupt controller.
+ - interrupts: Interrupt specifier for the controllers interrupt.
+
+
+Please refer to gpio.txt in this directory for details of the common GPIO
+bindings used by client devices.
+
+Example: PCF8575 I/O expander node
+
+ pcf8575: gpio@20 {
+ compatible = "nxp,pcf8575";
+ reg = <0x20>;
+ interrupt-parent = <&irqpin2>;
+ interrupts = <3 0>;
+ gpio-controller;
+ #gpio-cells = <2>;
+ interrupt-controller;
+ #interrupt-cells = <2>;
+ };
diff --git a/drivers/gpio/gpio-pcf857x.c b/drivers/gpio/gpio-pcf857x.c
index 9e61bb0..474f8ce 100644
--- a/drivers/gpio/gpio-pcf857x.c
+++ b/drivers/gpio/gpio-pcf857x.c
@@ -26,6 +26,8 @@
#include
#include
#include
+#include
+#include
#include
#include
#include
@@ -50,6 +52,27 @@ static const struct i2c_device_id pcf857x_id[] = {
};
MODULE_DEVICE_TABLE(i2c, pcf857x_id);
+#ifdef CONFIG_OF
+static const struct of_device_id pcf857x_of_table[] = {
+ { .compatible = "nxp,pcf8574", .data = (void *)8 },
+ { .compatible = "nxp,pcf8574a", .data = (void *)8 },
+ { .compatible = "nxp,pca8574", .data = (void *)8 },
+ { .compatible = "nxp,pca9670", .data = (void *)8 },
+ { .compatible = "nxp,pca9672", .data
Re: [PATCH] DMA: let filter functions of of_dma_simple_xlate possible check of_node
On Friday 23 August 2013 09:57:43 Stephen Warren wrote: > On 08/22/2013 07:29 PM, Richard Zhao wrote: > > On Fri, Aug 23, 2013 at 04:18:27AM +0800, Stephen Warren wrote: > >> On 08/21/2013 11:19 PM, Richard Zhao wrote: > >>> On Fri, Aug 02, 2013 at 10:00:00AM +0800, Richard Zhao wrote: > >>>> pass of_phandle_args dma_spec to dma_request_channel in > >>>> of_dma_simple_xlate, so the filter function could access of_node in > >>>> of_phandle_args. > >>>> > >>>> It also remove restriction of #dma-cells has to be one. > >>>> > >>>> Signed-off-by: Richard Zhao > >>>> --- > >>>> > >>>> drivers/dma/edma.c | 7 +-- > >>>> drivers/dma/of-dma.c | 10 -- > >>>> drivers/dma/omap-dma.c | 6 -- > >>>> 3 files changed, 13 insertions(+), 10 deletions(-) > >>> > >>> Hi Vinod, > >>> > >>> Can you please pick up this change? > >>> > >>> Hi Stephen, > >>> > >>> Can you please give a ack or reviewed-by etc? > >> > >> Hmm. Looking at the patch, I'm not sure it's right. > >> > >> This patch simply passes all the specfier args to the filter function, > >> and the code to check the equality of the of_node to the filter args is > >> still duplicated in each DMA driver. Instead, the DMA core should be > >> implementing the equality check, and only even calling the > >> driver-specific filter function for devices where the client's phandle > >> matches the DMA providing device's of_node handle. > > > > Filter function is called in dmaengine core code, independent of dt. > > The core code can still check if a dmaengine's driver was instantiated > from DT and take additional actions in that case. > > > And the reason why the driver has to write its own filter function is > > it has to store slave id there in its own way. > > I'm not saying don't call the driver's filter function, but rather that > the dmaengine core should perform the common checks before doing so. And it looks to me like the common case could even get rid of the driver's filter function: https://lkml.org/lkml/2013/5/15/270 https://lkml.org/lkml/2013/3/25/250 -- Regards, Laurent Pinchart -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] DMA: let filter functions of of_dma_simple_xlate possible check of_node
Hi Richard, (Dropping Dan Williams from the CC list as his e-mail address doesn't seem to be valid anymore) On Monday 26 August 2013 20:55:57 Richard Zhao wrote: > On Mon, Aug 26, 2013 at 02:17:43PM +0200, Laurent Pinchart wrote: > > On Friday 23 August 2013 09:57:43 Stephen Warren wrote: > > > On 08/22/2013 07:29 PM, Richard Zhao wrote: > > > > On Fri, Aug 23, 2013 at 04:18:27AM +0800, Stephen Warren wrote: > > > >> On 08/21/2013 11:19 PM, Richard Zhao wrote: > > > >>> On Fri, Aug 02, 2013 at 10:00:00AM +0800, Richard Zhao wrote: > > > >>>> pass of_phandle_args dma_spec to dma_request_channel in > > > >>>> of_dma_simple_xlate, so the filter function could access of_node in > > > >>>> of_phandle_args. > > > >>>> > > > >>>> It also remove restriction of #dma-cells has to be one. > > > >>>> > > > >>>> Signed-off-by: Richard Zhao > > > >>>> --- > > > >>>> > > > >>>> drivers/dma/edma.c | 7 +-- > > > >>>> drivers/dma/of-dma.c | 10 -- > > > >>>> drivers/dma/omap-dma.c | 6 -- > > > >>>> 3 files changed, 13 insertions(+), 10 deletions(-) > > > >>> > > > >>> Hi Vinod, > > > >>> > > > >>> Can you please pick up this change? > > > >>> > > > >>> Hi Stephen, > > > >>> > > > >>> Can you please give a ack or reviewed-by etc? > > > >> > > > >> Hmm. Looking at the patch, I'm not sure it's right. > > > >> > > > >> This patch simply passes all the specfier args to the filter > > > >> function, and the code to check the equality of the of_node to the > > > >> filter args is still duplicated in each DMA driver. Instead, the DMA > > > >> core should be implementing the equality check, and only even calling > > > >> the driver-specific filter function for devices where the client's > > > >> phandle matches the DMA providing device's of_node handle. > > > > > > > > Filter function is called in dmaengine core code, independent of dt. > > > > > > The core code can still check if a dmaengine's driver was instantiated > > > from DT and take additional actions in that case. > > > > > > > And the reason why the driver has to write its own filter function is > > > > it has to store slave id there in its own way. > > > > > > I'm not saying don't call the driver's filter function, but rather that > > > the dmaengine core should perform the common checks before doing so. > > > > And it looks to me like the common case could even get rid of the driver's > > filter function: > > > > https://lkml.org/lkml/2013/5/15/270 > > https://lkml.org/lkml/2013/3/25/250 > > For general case, the slave id is not staticly bind to a specific channel. Certainly not in all cases, but I think it's common enough to deserve a specific helper function. -- Regards, Laurent Pinchart -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v3] gpio: pcf857x: Add OF support
Hi Sylwester,
On Sunday 25 August 2013 10:16:40 Sylwester Nawrocki wrote:
> On 08/25/2013 02:26 AM, Laurent Pinchart wrote:
> > Add DT bindings for the pcf857x-compatible chips and parse the device
> > tree node in the driver.
> >
> > Signed-off-by: Laurent Pinchart
> > ---
> >
> > .../devicetree/bindings/gpio/gpio-pcf857x.txt | 71
> > drivers/gpio/gpio-pcf857x.c| 57 +---
> > 2 files changed, 119 insertions(+), 9 deletions(-)
> > create mode 100644
> > Documentation/devicetree/bindings/gpio/gpio-pcf857x.txt
> >
> > Changes since v2
> >
> > - Replace mention about interrupts software configuration in DT bindings
> >documentation with an explanation of the hardware configuration.
>
> [...]
>
> > @@ -257,14 +280,29 @@ fail:
> > static int pcf857x_probe(struct i2c_client *client,
> > const struct i2c_device_id *id)
> > {
> > - struct pcf857x_platform_data*pdata;
> > + struct pcf857x_platform_data*pdata = dev_get_platdata(&client-
> > >dev);
> > + struct device_node *np = client->dev.of_node;
> > struct pcf857x *gpio;
> > + unsigned intn_latch = 0;
> > + unsigned intngpio;
> > int status;
> >
> > - pdata = dev_get_platdata(&client->dev);
> > - if (!pdata) {
> > +#ifdef CONFIG_OF
> > + if (np) {
> > + const struct of_device_id *of_id;
> > +
> > + of_id = of_match_device(pcf857x_of_table,&client->dev);
> > + ngpio = (unsigned int)of_id->data;
>
> It is potentially unsafe, since of_match_device() can return NULL.
But if np isn't NULL the probe function will only get called if there's a
match in the OF table, so of_match_device() won't return NULL.
> > + } else
> > +#endif
> > + ngpio = id->driver_data;
>
> CodingStyle: braces are missing for the second part of the if statement.
>
>
> How about something along the lines of:
>
> const struct of_device_id *of_id;
>
> of_id = of_match_device(of_match_ptr(pcf857x_of_table), &client->dev);
> if (of_id)
> ngpio = (unsigned int)of_id->data;
> else
> ngpio = id->driver_data;
>
> to get rid of the #ifdef ?
Looks good to me, I'll post v4.
> But still DT has priority over platform data here.
>
> > +
> > + if (pdata)
> > + n_latch = pdata->n_latch;
> > + else if (IS_ENABLED(CONFIG_OF)&& np)
> > + of_property_read_u32(np, "pins-initial-state",&n_latch);
>
> And here platform data has priority over DT.
We can't have both platform data and DT, but I'll change the order anyway.
> > + else
> > dev_dbg(&client->dev, "no platform data\n");
> > - }
> > /* Allocate, initialize, and register this gpio_chip. */
> > gpio = devm_kzalloc(&client->dev, sizeof(*gpio), GFP_KERNEL);
--
Regards,
Laurent Pinchart
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH v4] gpio: pcf857x: Add OF support
Add DT bindings for the pcf857x-compatible chips and parse the device
tree node in the driver.
Signed-off-by: Laurent Pinchart
---
.../devicetree/bindings/gpio/gpio-pcf857x.txt | 71 ++
drivers/gpio/gpio-pcf857x.c| 54 +---
2 files changed, 116 insertions(+), 9 deletions(-)
create mode 100644 Documentation/devicetree/bindings/gpio/gpio-pcf857x.txt
Changes since v3:
- Get rid of the #ifdef CONFIG_OF in the probe function
- Give DT node priority over platform data
Changes since v2
- Replace mention about interrupts software configuration in DT bindings
documentation with an explanation of the hardware configuration.
diff --git a/Documentation/devicetree/bindings/gpio/gpio-pcf857x.txt
b/Documentation/devicetree/bindings/gpio/gpio-pcf857x.txt
new file mode 100644
index 000..d261391
--- /dev/null
+++ b/Documentation/devicetree/bindings/gpio/gpio-pcf857x.txt
@@ -0,0 +1,71 @@
+* PCF857x-compatible I/O expanders
+
+The PCF857x-compatible chips have "quasi-bidirectional" I/O pins that can be
+driven high by a pull-up current source or driven low to ground. This combines
+the direction and output level into a single bit per pin, which can't be read
+back. We can't actually know at initialization time whether a pin is configured
+(a) as output and driving the signal low/high, or (b) as input and reporting a
+low/high value, without knowing the last value written since the chip came out
+of reset (if any). The only reliable solution for setting up pin direction is
+thus to do it explicitly.
+
+Required Properties:
+
+ - compatible: should be one of the following.
+- "maxim,max7328": For the Maxim MAX7378
+- "maxim,max7329": For the Maxim MAX7329
+- "nxp,pca8574": For the NXP PCA8574
+- "nxp,pca8575": For the NXP PCA8575
+- "nxp,pca9670": For the NXP PCA9670
+- "nxp,pca9671": For the NXP PCA9671
+- "nxp,pca9672": For the NXP PCA9672
+- "nxp,pca9673": For the NXP PCA9673
+- "nxp,pca9674": For the NXP PCA9674
+- "nxp,pca9675": For the NXP PCA9675
+- "nxp,pcf8574": For the NXP PCF8574
+- "nxp,pcf8574a": For the NXP PCF8574A
+- "nxp,pcf8575": For the NXP PCF8575
+- "ti,tca9554": For the TI TCA9554
+
+ - reg: I2C slave address.
+
+ - gpio-controller: Marks the device node as a gpio controller.
+ - #gpio-cells: Should be 2. The first cell is the GPIO number and the second
+cell specifies GPIO flags, as defined in . Only
the
+GPIO_ACTIVE_HIGH and GPIO_ACTIVE_LOW flags are supported.
+
+Optional Properties:
+
+ - pins-initial-state: Bitmask that specifies the initial state of each pin.
+ When a bit is set to zero, the corresponding pin will be initialized to the
+ input (pulled-up) state. When the bit is set to one, the pin will be
+ initialized the the low-level output state. If the property is not specified
+ all pins will be initialized to the input state.
+
+ The I/O expander can detect input state changes, and thus optionally act as
+ an interrupt controller. When the expander interrupt pin is connected all the
+ following properties must be set. For more information please see the
+ interrupt controller device tree bindings documentation available at
+ Documentation/devicetree/bindings/interrupt-controller/interrupts.txt.
+
+ - interrupt-controller: Identifies the node as an interrupt controller.
+ - #interrupt-cells: Number of cells to encode an interrupt source, shall be
2.
+ - interrupt-parent: phandle of the parent interrupt controller.
+ - interrupts: Interrupt specifier for the controllers interrupt.
+
+
+Please refer to gpio.txt in this directory for details of the common GPIO
+bindings used by client devices.
+
+Example: PCF8575 I/O expander node
+
+ pcf8575: gpio@20 {
+ compatible = "nxp,pcf8575";
+ reg = <0x20>;
+ interrupt-parent = <&irqpin2>;
+ interrupts = <3 0>;
+ gpio-controller;
+ #gpio-cells = <2>;
+ interrupt-controller;
+ #interrupt-cells = <2>;
+ };
diff --git a/drivers/gpio/gpio-pcf857x.c b/drivers/gpio/gpio-pcf857x.c
index 9e61bb0..f652bb4 100644
--- a/drivers/gpio/gpio-pcf857x.c
+++ b/drivers/gpio/gpio-pcf857x.c
@@ -26,6 +26,8 @@
#include
#include
#include
+#include
+#include
#include
#include
#include
@@ -50,6 +52,27 @@ static const struct i2c_device_id pcf857x_id[] = {
};
MODULE_DEVICE_TABLE(i2c, pcf857x_id);
+#ifdef CONFIG_OF
+static const struct of_device_id pcf857x_of_table[] = {
+ { .compatible = "nxp,pcf8574", .data = (void *)8 },
+ { .compatible = "nxp,pcf8574a", .data = (void *)8 },
+ { .compatible = "nxp,pca8574", .data = (void *)8 },
+
[PATCH v5] gpio: pcf857x: Add OF support
Add DT bindings for the pcf857x-compatible chips and parse the device
tree node in the driver.
Signed-off-by: Laurent Pinchart
---
.../devicetree/bindings/gpio/gpio-pcf857x.txt | 71 ++
drivers/gpio/gpio-pcf857x.c| 44 +++---
2 files changed, 107 insertions(+), 8 deletions(-)
create mode 100644 Documentation/devicetree/bindings/gpio/gpio-pcf857x.txt
Changes since v4:
- Don't try to get ngpio from of_device_id data, we already get it from
i2c_device_id
Changes since v3:
- Get rid of the #ifdef CONFIG_OF in the probe function
- Give DT node priority over platform data
Changes since v2:
- Replace mention about interrupts software configuration in DT bindings
documentation with an explanation of the hardware configuration.
diff --git a/Documentation/devicetree/bindings/gpio/gpio-pcf857x.txt
b/Documentation/devicetree/bindings/gpio/gpio-pcf857x.txt
new file mode 100644
index 000..d261391
--- /dev/null
+++ b/Documentation/devicetree/bindings/gpio/gpio-pcf857x.txt
@@ -0,0 +1,71 @@
+* PCF857x-compatible I/O expanders
+
+The PCF857x-compatible chips have "quasi-bidirectional" I/O pins that can be
+driven high by a pull-up current source or driven low to ground. This combines
+the direction and output level into a single bit per pin, which can't be read
+back. We can't actually know at initialization time whether a pin is configured
+(a) as output and driving the signal low/high, or (b) as input and reporting a
+low/high value, without knowing the last value written since the chip came out
+of reset (if any). The only reliable solution for setting up pin direction is
+thus to do it explicitly.
+
+Required Properties:
+
+ - compatible: should be one of the following.
+- "maxim,max7328": For the Maxim MAX7378
+- "maxim,max7329": For the Maxim MAX7329
+- "nxp,pca8574": For the NXP PCA8574
+- "nxp,pca8575": For the NXP PCA8575
+- "nxp,pca9670": For the NXP PCA9670
+- "nxp,pca9671": For the NXP PCA9671
+- "nxp,pca9672": For the NXP PCA9672
+- "nxp,pca9673": For the NXP PCA9673
+- "nxp,pca9674": For the NXP PCA9674
+- "nxp,pca9675": For the NXP PCA9675
+- "nxp,pcf8574": For the NXP PCF8574
+- "nxp,pcf8574a": For the NXP PCF8574A
+- "nxp,pcf8575": For the NXP PCF8575
+- "ti,tca9554": For the TI TCA9554
+
+ - reg: I2C slave address.
+
+ - gpio-controller: Marks the device node as a gpio controller.
+ - #gpio-cells: Should be 2. The first cell is the GPIO number and the second
+cell specifies GPIO flags, as defined in . Only
the
+GPIO_ACTIVE_HIGH and GPIO_ACTIVE_LOW flags are supported.
+
+Optional Properties:
+
+ - pins-initial-state: Bitmask that specifies the initial state of each pin.
+ When a bit is set to zero, the corresponding pin will be initialized to the
+ input (pulled-up) state. When the bit is set to one, the pin will be
+ initialized the the low-level output state. If the property is not specified
+ all pins will be initialized to the input state.
+
+ The I/O expander can detect input state changes, and thus optionally act as
+ an interrupt controller. When the expander interrupt pin is connected all the
+ following properties must be set. For more information please see the
+ interrupt controller device tree bindings documentation available at
+ Documentation/devicetree/bindings/interrupt-controller/interrupts.txt.
+
+ - interrupt-controller: Identifies the node as an interrupt controller.
+ - #interrupt-cells: Number of cells to encode an interrupt source, shall be
2.
+ - interrupt-parent: phandle of the parent interrupt controller.
+ - interrupts: Interrupt specifier for the controllers interrupt.
+
+
+Please refer to gpio.txt in this directory for details of the common GPIO
+bindings used by client devices.
+
+Example: PCF8575 I/O expander node
+
+ pcf8575: gpio@20 {
+ compatible = "nxp,pcf8575";
+ reg = <0x20>;
+ interrupt-parent = <&irqpin2>;
+ interrupts = <3 0>;
+ gpio-controller;
+ #gpio-cells = <2>;
+ interrupt-controller;
+ #interrupt-cells = <2>;
+ };
diff --git a/drivers/gpio/gpio-pcf857x.c b/drivers/gpio/gpio-pcf857x.c
index 9e61bb0..864dd8c 100644
--- a/drivers/gpio/gpio-pcf857x.c
+++ b/drivers/gpio/gpio-pcf857x.c
@@ -26,6 +26,8 @@
#include
#include
#include
+#include
+#include
#include
#include
#include
@@ -50,6 +52,27 @@ static const struct i2c_device_id pcf857x_id[] = {
};
MODULE_DEVICE_TABLE(i2c, pcf857x_id);
+#ifdef CONFIG_OF
+static const struct of_device_id pcf857x_of_table[] = {
+ { .compatible = "nxp,pcf8574" },
+ { .compatible = "nxp,pcf8574a"
Re: [PATCH v2 3/3] gpio: pcf857x: Add OF support
Hi Mark,
On Tuesday 27 August 2013 11:39:49 Mark Rutland wrote:
> On Sat, Aug 24, 2013 at 03:13:11PM +0100, Tomasz Figa wrote:
> > On Saturday 24 of August 2013 02:54:07 Laurent Pinchart wrote:
> > > On Saturday 24 August 2013 02:41:59 Tomasz Figa wrote:
> > > > On Tuesday 20 of August 2013 01:04:54 Laurent Pinchart wrote:
> > > > > Add DT bindings for the pcf857x-compatible chips and parse the
> > > > > device tree node in the driver.
> > > > >
> > > > > Signed-off-by: Laurent Pinchart
> > > > >
> > > > > ---
> > > > >
> > > > > .../devicetree/bindings/gpio/gpio-pcf857x.txt | 71 +++
> > > > > drivers/gpio/gpio-pcf857x.c| 57 +--
> > > > > 2 files changed, 119 insertions(+), 9 deletions(-)
> > > > > create mode 100644
> > > > > Documentation/devicetree/bindings/gpio/gpio-pcf857x.txt
> > > > >
> > > > > diff --git a/Documentation/devicetree/bindings/gpio/gpio-pcf857x.txt
> > > > > b/Documentation/devicetree/bindings/gpio/gpio-pcf857x.txt new file
> > > > > mode 100644
> > > > > index 000..df94462
> > > > > --- /dev/null
> > > > > +++ b/Documentation/devicetree/bindings/gpio/gpio-pcf857x.txt
> > > > > @@ -0,0 +1,71 @@
> > >
> > > [snip]
> > >
> > > > > + - pins-initial-state: Bitmask that specifies the initial state of
> > > > > +each pin. When a bit is set to zero, the corresponding pin will
> > > > > be
> > > > > +initialized to the input (pulled-up) state. When the bit is
> > > > > set to +one, the pin will be initialized the the low-level
> > > > > output state. If +the property is not specified all pins will
> > > > > be initialized to the +input state.
> > > >
> > > > Hmm, do you actually need to know whether those pins are outputs or
> > > > inputs before they get used for first time? I believe any driver
> > > > using GPIO will call gpio_direction_{in,out}put() before it starts
> > > > using the pin, which will initialize the pin to a known state.
> > > >
> > > > What I'd suggest is making the driver handle this by having a bit mask
> > > > that marks states of pins as defined and flagging all pins as
> > > > undefined by default. Then any call to gpio_direction_output() or
> > > > _input() would mark it as defined and direction of the pin could be
> > > > stored in internal driver structures.
> > >
> > > The problem is that all pins are controlled through a single I2C write.
> > > Setting the direction of a pin will set the direction of all other pins.
> > > I thus need to know what the initial settings are to avoid glitches.
>
> I guess it's not possible to read the initial state from the hardware?
I wish. Unfortunately it can only be written.
> > Oh, that's a funny hardware, isn't it? :)
> >
> > Well, I guess it can't be helped then. Sorry for the noise.
> >
> > > > > + The I/O expander can detect input state changes, and thus
> > > > > optionally
> > > > > + act as an interrupt controller. When interrupts support is
> > > > > desired
> > > >
> > > > I don't like this statement. Device tree should represent what the
> > > > device allows you to do, not what you want the device to do.
> > > >
> > > > My opinion on this is that if the chip supports interrupts then it
> > > > should always be an interrupt-controller (unless its interrupt pin is
> > > > not wired on the board, but this still conforms to what I wrote
> > > > above).
> > >
> > > I agree. What about the following text then ?
> > >
> > > The I/O expander can detect input state changes, and thus optionally act
> > > as an interrupt controller. When the expander interrupt pin is
> > > connected all the following properties must be set. For more
> > > information please see the interrupt controller device tree bindings
> > > documentation available at
> > > Documentation/devicetree/bindings/interrupt-controller/interrupts.txt.
> >
> > Sounds good.
> >
> > > > > diff --git a/drivers/gpio/gpio-pcf857x.c
> > > > > b/drivers/gpio/gpio-pcf857x.c
> > > > > index 070e81f..5
Re: [PATCH v5] gpio: pcf857x: Add OF support
Hi Tomasz, On Tuesday 27 August 2013 13:55:00 Tomasz Figa wrote: > On Tuesday 27 of August 2013 14:00:24 Archit Taneja wrote: > > On Tuesday 27 August 2013 01:44 PM, Tomasz Figa wrote: > > > On Tuesday 27 of August 2013 10:02:39 Laurent Pinchart wrote: > > >> Add DT bindings for the pcf857x-compatible chips and parse the device > > >> tree node in the driver. > > >> > > >> Signed-off-by: Laurent Pinchart > > >> --- > > >> > > >> .../devicetree/bindings/gpio/gpio-pcf857x.txt | 71 + > > >> drivers/gpio/gpio-pcf857x.c| 44 ++--- > > >> 2 files changed, 107 insertions(+), 8 deletions(-) > > >> create mode 100644 > > >> Documentation/devicetree/bindings/gpio/gpio-pcf857x.txt > > >> > > >> Changes since v4: > > >> > > >> - Don't try to get ngpio from of_device_id data, we already get it > > >> from i2c_device_id > > > > > > Hmm, I'm not sure how this is supposed to work. > > > > > > How does the I2C core resolve OF compatible name to particular entry in > > > id_table? I believe it simply passes NULL as the second argument of > > > .probe() if the device is instantiated based on OF compatible string > > > and not one in the legacy ID table. > > > > It doesn't pass the second argument as NULL. If you look at > > i2c_device_probe() in drivers/i2c/i2c-core.c, the second argument to > > probe is passed as: i2c_match_id(driver->id_table, client) > > > > This will extract the i2c_device_id pointer from the id_table. > > Yes, there is a chance that it will not return NULL, but I think that > relying on this is rather flawed. > > If you look at the whole code path, you can see that it's only a > coincidence that this works. See the execution flow: > - I2C adapter driver calls of_i2c_register_devices(), > - of_i2c_register_devices() calls of_modalias_node() for every device on > this bus, > - of_modalias_node() stores the second substring of compatible string > separated by a comma, if there is one or the whole compatible otherwise to > the output buffer (type field of i2c_board_info struct, as passed by > of_i2c_register_devices()), > - of_i2c_register_devices() then calls i2c_new_device() with the resulting > info struct, > - i2c_new_device() takes info->type and copies its contents to client->name > - then a bit later, I2C core calls i2c_match_id(), which does matching of > client->name against id_table of the driver and the resulting i2c_device_id > entry (or NULL) is then passed to driver's .probe() callback. > > So if it happens that compatible is not equal to ", I2C table>", then the matching will fail and NULL will be passed. The driver should support the same chip models reardless of whether it's used with or without DT. If an entry in the OF table has no corresponding entry in the I2C table I would consider that as a driver bug. It would be caught early, as the driver would crash at probe time, so it will likely not make it to mainline (unless we merge untested code, but that's another issue :-)). > [CCing Wolfram and Grant, as they should now more about this behavior and > whether it's intentional or no] -- Regards, Laurent Pinchart -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/5] mfd: asic3: Remove .set_pwr() callback
Hi Jingoo,
Thank you for the patch.
On Wednesday 28 August 2013 14:52:16 Jingoo Han wrote:
> Since 3af9d15 "mmc: tmio-mmc: Remove .set_pwr() callback from
> platform data", .set_pwr() callback is removed from platform data.
> Thus, .set_pwr() is not used anymore. Also, this patch fixes
> the following build error and warning.
>
> drivers/mfd/asic3.c:724:2: error: unknown field 'set_pwr' specified in
> initializer drivers/mfd/asic3.c:724:2: warning: initialization makes
> integer from pointer without a cast [enabled by default]
> drivers/mfd/asic3.c:724:2: warning: (near initialization for
> 'asic3_mmc_data.capabilities' [enabled by default]
My bad, it looks like I've overlooked a few users of the .set_pwr() field :-/
Sorry about that.
> Signed-off-by: Jingoo Han
> Cc: Laurent Pinchart
> Cc: Guennadi Liakhovetski
> Cc: Ian Molton
> Cc: Chris Ball
> ---
> drivers/mfd/asic3.c |8
> 1 file changed, 8 deletions(-)
>
> diff --git a/drivers/mfd/asic3.c b/drivers/mfd/asic3.c
> index fa22154..7a6f713 100644
> --- a/drivers/mfd/asic3.c
> +++ b/drivers/mfd/asic3.c
> @@ -705,13 +705,6 @@ static struct mfd_cell asic3_cell_ds1wm = {
> .resources = ds1wm_resources,
> };
>
> -static void asic3_mmc_pwr(struct platform_device *pdev, int state)
> -{
> - struct asic3 *asic = dev_get_drvdata(pdev->dev.parent);
> -
> - tmio_core_mmc_pwr(asic->tmio_cnf, 1 - asic->bus_shift, state);
> -}
> -
I don't think blindly removing the function is the right fix, as it seems to
be needed. Looking at the whole series, I believe we should just revert
3af9d15 "mmc: tmio-mmc: Remove .set_pwr() callback from platform data"
instead.
> static void asic3_mmc_clk_div(struct platform_device *pdev, int state)
> {
> struct asic3 *asic = dev_get_drvdata(pdev->dev.parent);
> @@ -721,7 +714,6 @@ static void asic3_mmc_clk_div(struct platform_device
> *pdev, int state)
>
> static struct tmio_mmc_data asic3_mmc_data = {
> .hclk = 24576000,
> - .set_pwr= asic3_mmc_pwr,
> .set_clk_div= asic3_mmc_clk_div,
> };
--
Regards,
Laurent Pinchart
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/5] mfd: asic3: Remove .set_pwr() callback
Hi Lee, (Dropping Ian Molton from the CC list as the e- mail address isn't valid anymore) On Wednesday 28 August 2013 08:41:26 Lee Jones wrote: > > > Since 3af9d15 "mmc: tmio-mmc: Remove .set_pwr() callback from > > > platform data", .set_pwr() callback is removed from platform data. > > > Thus, .set_pwr() is not used anymore. Also, this patch fixes > > > the following build error and warning. > > > > > > drivers/mfd/asic3.c:724:2: error: unknown field 'set_pwr' specified in > > > initializer drivers/mfd/asic3.c:724:2: warning: initialization makes > > > integer from pointer without a cast [enabled by default] > > > drivers/mfd/asic3.c:724:2: warning: (near initialization for > > > 'asic3_mmc_data.capabilities' [enabled by default] > > > > My bad, it looks like I've overlooked a few users of the .set_pwr() field > > :-/ Sorry about that. > > > > > I don't think blindly removing the function is the right fix, as it seems > > to be needed. Looking at the whole series, I believe we should just > > revert 3af9d15 "mmc: tmio-mmc: Remove .set_pwr() callback from platform > > data" instead. > > No need to revert it, as it's not in Mainline yet. Just request for it > to be removed from Chris' tree. I spupose it depends on whether Chris has provided a stable branch with the patch included. I'm fine with both, the commit just needs to be undone one way or another. -- Regards, Laurent Pinchart -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v5] gpio: pcf857x: Add OF support
On Tuesday 27 August 2013 19:18:55 Wolfram Sang wrote: > > The driver should support the same chip models reardless of whether it's > > used with or without DT. If an entry in the OF table has no corresponding > > entry in the I2C table I would consider that as a driver bug. > > Linus Walleij posted a patch to support DT only probing, but too many > side-effects showed up. Some were fixable (probe regressions) and some > need more work, if feasible at all. Most prominent is that runtime > instantiation of i2c slaves currently needs an entry in the i2c table. Linus, I'd like to get this in v3.12 if it's not too late. Could you please provide your input ? -- Regards, Laurent Pinchart -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/5] mfd: asic3: Remove .set_pwr() callback
Hi Chris, On Thursday 29 August 2013 07:42:26 Kevin Hilman wrote: > On Wed, Aug 28, 2013 at 12:51 AM, Laurent Pinchart wrote: > > On Wednesday 28 August 2013 08:41:26 Lee Jones wrote: > >> > > Since 3af9d15 "mmc: tmio-mmc: Remove .set_pwr() callback from > >> > > platform data", .set_pwr() callback is removed from platform data. > >> > > Thus, .set_pwr() is not used anymore. Also, this patch fixes > >> > > the following build error and warning. > >> > > > >> > > drivers/mfd/asic3.c:724:2: error: unknown field 'set_pwr' specified > >> > > in initializer drivers/mfd/asic3.c:724:2: warning: initialization > >> > > makes integer from pointer without a cast [enabled by default] > >> > > drivers/mfd/asic3.c:724:2: warning: (near initialization for > >> > > 'asic3_mmc_data.capabilities' [enabled by default] > >> > > >> > My bad, it looks like I've overlooked a few users of the .set_pwr() > >> > field > >> > > >> > :-/ Sorry about that. > >> > >> > >> > >> > I don't think blindly removing the function is the right fix, as it > >> > seems to be needed. Looking at the whole series, I believe we should > >> > just revert 3af9d15 "mmc: tmio-mmc: Remove .set_pwr() callback from > >> > platform data" instead. > >> > >> No need to revert it, as it's not in Mainline yet. Just request for it > >> to be removed from Chris' tree. > > > > I spupose it depends on whether Chris has provided a stable branch with > > the patch included. I'm fine with both, the commit just needs to be undone > > one way or another. > > Any progress on this? linux-next builds are still failing for the > following defconfigs because of this removal: Chris, could you please drop 3af9d15 "mmc: tmio-mmc: Remove .set_pwr() callback from platform data" from your tree or revert it ? > Failed defconfigs: > arm-magician_defconfig > arm-eseries_pxa_defconfig > > Errors: > > arm-magician_defconfig > drivers/mfd/asic3.c:724:2: error: unknown field 'set_pwr' specified in > initializer > > arm-eseries_pxa_defconfig > drivers/mfd/t7l66xb.c:146:2: error: unknown field 'set_pwr' specified > in initializer > drivers/mfd/tc6387xb.c:123:2: error: unknown field 'set_pwr' specified > in initializer > drivers/mfd/tc6393xb.c:381:2: error: unknown field 'set_pwr' specified > in initializer -- Regards, Laurent Pinchart -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 4/6] ARM: shmobile: emev2: Define SMU clock DT bindings
gt; +Example of clock-tree description:
> > +
> > + This describes a clock path in the clock tree
> > + c32ki -> pll3_fo -> usia_u0_sclkdiv -> usia_u0_sclk
> > +
> > +smu {
> > + compatible = "renesas,emev2-smu";
> > + reg = <0xe011 0x1>;
> > + #address-cells = <2>;
> > + #size-cells = <0>;
> > +
> > + c32ki: c32ki {
> > + compatible = "fixed-clock";
> > + clock-frequency = <32768>;
> > + #clock-cells = <0>;
> > + };
> > + pll3_fo: pll3_fo {
> > + compatible = "fixed-factor-clock";
> > + clocks = <&c32ki>;
> > + clock-div = <1>;
> > + clock-mult = <7000>;
> > + #clock-cells = <0>;
> > + };
> > + usia_u0_sclkdiv: usia_u0_sclkdiv {
> > + compatible = "renesas,emev2-smu-clkdiv";
> > + reg = <0x610 0>;
> > + clocks = <&pll3_fo>;
> > + #clock-cells = <0>;
> > + };
> > + usia_u0_sclk: usia_u0_sclk {
> > + compatible = "renesas,emev2-smu-gclk";
> > + reg = <0x4a0 1>;
> > + clocks = <&usia_u0_sclkdiv>;
> > + #clock-cells = <0>;
> > + };
> > +};
> > +
There's an extra blank line at the end of the file.
--
Regards,
Laurent Pinchart
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 5/6] clk: emev2: Add support for emev2 SMU clocks with DT
Hi Yoshii-san, Thank you for the patch. (CC'ing LAMK as a generic CCF question follows) On Tuesday 01 October 2013 18:15:26 Magnus Damm wrote: > On Tue, Sep 24, 2013 at 1:15 PM, wrote: > > Common clock framework version of emev2 clock support. > > smu_clkdiv and smu_gclk are handled. > > So far, reparent is not implemented, and is fixed to index #0. > > SMU and small numbers of clocks are described in emev2.dtsi. > > > > That function and numbers of clocks are equivalent to current > > sh-clkfwk version. It is just enough to run kzm9d-reference. > > > > Signed-off-by: Takashi Yoshii > > --- > > > > arch/arm/boot/dts/emev2.dtsi | 84 +++ > > drivers/clk/Makefile | 2 + > > drivers/clk/shmobile/Makefile| 5 ++ > > drivers/clk/shmobile/clk-emev2.c | 104 ++ > > 4 files changed, 195 insertions(+) > > create mode 100644 drivers/clk/shmobile/Makefile > > create mode 100644 drivers/clk/shmobile/clk-emev2.c > > Hi Yoshii-san, > > Thanks for your efforts on this. I'm very pleased to see that you describe > the clock topology using DT. What is the generally accepted practice when an IP core provides a large number of clocks, with either one register or one register bit dedicated to each clock ? Should each clock be described as one DT node, or should the IP core be described by a single DT node ? I also see both platforms using CLK_OF_DECLARE and platforms calling a clock init function provided by drivers/clk/.c in the SoC setup code in arch/arm/. What is the preferred practice there ? > In general I think your patch looks fine, but I have some comment related to > the multiplatform integration, please see below. > > > diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile > > index 7b11106..3e64ac4 100644 > > --- a/drivers/clk/Makefile > > +++ b/drivers/clk/Makefile > > @@ -32,6 +32,8 @@ obj-$(CONFIG_ARCH_VT8500) += clk-vt8500.o > > > > obj-$(CONFIG_ARCH_ZYNQ)+= zynq/ > > obj-$(CONFIG_ARCH_TEGRA) += tegra/ > > obj-$(CONFIG_PLAT_SAMSUNG) += samsung/ > > > > +obj-$(CONFIG_ARCH_SHMOBILE)+= shmobile/ > > +obj-$(CONFIG_ARCH_SHMOBILE_MULTI) += shmobile/ > > Here I believe it is enough that you only use > CONFIG_ARCH_SHMOBILE_MULTI. Building common clocks to coexist with the > old legacy board code does not make any sense IMO. If you think it > makes sense for some reason, please explain why. =) > > > diff --git a/drivers/clk/shmobile/Makefile b/drivers/clk/shmobile/Makefile > > new file mode 100644 > > index 000..6a26eb6 > > --- /dev/null > > +++ b/drivers/clk/shmobile/Makefile > > @@ -0,0 +1,5 @@ > > +ifeq ($(CONFIG_COMMON_CLK), y) > > +obj-$(CONFIG_ARCH_EMEV2) += clk-emev2.o > > +endif > > I don't think you would need the above ifeq/endif wrapper if you only > used CONFIG_ARCH_SHMOBILE_MULTI above. > > Apart from that it looks good to me. And, yes, I have tested this on > my KZM9D board together with multiplatform and it works very well! -- Regards, Laurent Pinchart -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 05/26] omap3isp: Make isp_video_buffer_prepare_user() use get_user_pages_fast()
Hi Jan,
Thank you for the patch.
On Wednesday 02 October 2013 16:27:46 Jan Kara wrote:
> CC: Laurent Pinchart
> CC: linux-me...@vger.kernel.org
> Signed-off-by: Jan Kara
Acked-by: Laurent Pinchart
Could you briefly explain where you're headed with this ? The V4L2 subsystem
has suffered for quite a long time from potentiel AB-BA deadlocks, due the
drivers taking the mmap_sem semaphore while holding the V4L2 buffers queue
lock in the code path below, and taking them in reverse order in the mmap()
path (as the mm core takes the mmap_sem semaphore before calling the mmap()
operation). We've solved that by releasing the V4L2 buffers queue lock before
taking the mmap_sem lock below and taking it back after releasing the mmap_sem
lock. Having an explicit indication that the mmap_sem lock was taken helped
keeping the problem in mind. I'm not against hiding it in
get_user_pages_fast(), but I'd like to know what the next step is. Maybe it
would also be worth it adding a /* get_user_pages_fast() takes the mmap_sem */
comment before the call ?
> ---
> drivers/media/platform/omap3isp/ispqueue.c | 10 +++---
> 1 file changed, 3 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/media/platform/omap3isp/ispqueue.c
> b/drivers/media/platform/omap3isp/ispqueue.c index
> e15f01342058..bed380395e6c 100644
> --- a/drivers/media/platform/omap3isp/ispqueue.c
> +++ b/drivers/media/platform/omap3isp/ispqueue.c
> @@ -331,13 +331,9 @@ static int isp_video_buffer_prepare_user(struct
> isp_video_buffer *buf) if (buf->pages == NULL)
> return -ENOMEM;
>
> - down_read(¤t->mm->mmap_sem);
> - ret = get_user_pages(current, current->mm, data & PAGE_MASK,
> - buf->npages,
> - buf->vbuf.type == V4L2_BUF_TYPE_VIDEO_CAPTURE, 0,
> - buf->pages, NULL);
> - up_read(¤t->mm->mmap_sem);
> -
> + ret = get_user_pages_fast(data & PAGE_MASK, buf->npages,
> + buf->vbuf.type == V4L2_BUF_TYPE_VIDEO_CAPTURE,
> + buf->pages);
> if (ret != buf->npages) {
> buf->npages = ret < 0 ? 0 : ret;
> isp_video_buffer_cleanup(buf);
--
Regards,
Laurent Pinchart
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] fbdev: sh_mobile_hdmi: Use devm_kzalloc()
Hi Sangjung,
Thank you for the patch.
On Thursday 03 October 2013 23:04:27 Sangjung Woo wrote:
> Use devm_kzalloc() instead of kzalloc() in order to be free
> automatically. This makes cleanup paths more simple.
>
> Signed-off-by: Sangjung Woo
Acked-by: Laurent Pinchart
Tomi, could you please pick it up ?
> ---
> drivers/video/sh_mobile_hdmi.c |7 ++-
> 1 file changed, 2 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/video/sh_mobile_hdmi.c b/drivers/video/sh_mobile_hdmi.c
> index bfe4728..64bd0bf 100644
> --- a/drivers/video/sh_mobile_hdmi.c
> +++ b/drivers/video/sh_mobile_hdmi.c
> @@ -1290,7 +1290,7 @@ static int __init sh_hdmi_probe(struct platform_device
> *pdev) }
> }
>
> - hdmi = kzalloc(sizeof(*hdmi), GFP_KERNEL);
> + hdmi = devm_kzalloc(&pdev->dev, sizeof(*hdmi), GFP_KERNEL);
> if (!hdmi) {
> dev_err(&pdev->dev, "Cannot allocate device data\n");
> return -ENOMEM;
> @@ -1304,7 +1304,7 @@ static int __init sh_hdmi_probe(struct platform_device
> *pdev) if (IS_ERR(hdmi->hdmi_clk)) {
> ret = PTR_ERR(hdmi->hdmi_clk);
> dev_err(&pdev->dev, "Unable to get clock: %d\n", ret);
> - goto egetclk;
> + return ret;
> }
>
> /* select register access functions */
> @@ -1407,8 +1407,6 @@ ereqreg:
> clk_disable(hdmi->hdmi_clk);
> erate:
> clk_put(hdmi->hdmi_clk);
> -egetclk:
> - kfree(hdmi);
>
> return ret;
> }
> @@ -1433,7 +1431,6 @@ static int __exit sh_hdmi_remove(struct
> platform_device *pdev) iounmap(hdmi->htop1);
> iounmap(hdmi->base);
> release_mem_region(res->start, resource_size(res));
> - kfree(hdmi);
>
> return 0;
> }
--
Regards,
Laurent Pinchart
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: i2c: introduce i2c helper i2c_find_client_by_name()
On Friday 12 July 2013 12:00:47 Wolfram Sang wrote: > > Briefly looking into ACPI tables we have and mechanisms that we can > > use in ACPI case, I doubt we may apply all the ideas, probably some of > > them, though I didn't get yet where to read about in details. What I > > could say now is that the patch provided by Bin Gao is definitely no > > go. > > Laurent explained to me what V4L did and now does. It used to be the way > tha V4L drivers did register I2C slaves according to platform_data. Now, > with DT the slaves get instanciated earlier, so they now use notifiers > to know when the slaves are in place. Something like this should > probably be done here, too, instead of unregistering and re-registering. This will be available in v3.11 in drivers/media/v4l2-core/v4l2-async.c. -- Regards, Laurent Pinchart signature.asc Description: This is a digitally signed message part.
Re: i2c: introduce i2c helper i2c_find_client_by_name()
Hi Andy, On Friday 12 July 2013 14:29:16 Andy Shevchenko wrote: > On Fri, Jul 12, 2013 at 2:00 PM, Wolfram Sang wrote: > >> Briefly looking into ACPI tables we have and mechanisms that we can > >> use in ACPI case, I doubt we may apply all the ideas, probably some of > >> them, though I didn't get yet where to read about in details. What I > >> could say now is that the patch provided by Bin Gao is definitely no > >> go. > > > > Laurent explained to me what V4L did and now does. It used to be the way > > tha V4L drivers did register I2C slaves according to platform_data. Now, > > with DT the slaves get instanciated earlier, so they now use notifiers > > to know when the slaves are in place. Something like this should > > probably be done here, too, instead of unregistering and re-registering. > > Yes, seems right way to go. > I think ACPI case can use V4L2 async API somehow, though it has its > own event model. > I'll talk to Sakari Ailus to sync. Do you have any pointer to the relevant parts of the ACPI specification ? -- Regards, Laurent Pinchart -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] [media] media/v4l2: VIDEO_RENESAS_VSP1 should depend on HAS_DMA
Hi Geert, On Friday 06 September 2013 19:07:50 Geert Uytterhoeven wrote: > On Fri, Sep 6, 2013 at 5:20 PM, Laurent Pinchart wrote: > > On Friday 06 September 2013 14:43:56 Geert Uytterhoeven wrote: > >> If NO_DMA=y: > >> > >> warning: (... && VIDEO_RENESAS_VSP1 && ...) selects VIDEOBUF2_DMA_CONTIG > >> which has unmet direct dependencies (MEDIA_SUPPORT && HAS_DMA) > >> > >> drivers/media/v4l2-core/videobuf2-dma-contig.c: In function > >> ‘vb2_dc_mmap’: > >> drivers/media/v4l2-core/videobuf2-dma-contig.c:202: error: implicit > >> declaration of function ‘dma_mmap_coherent’ > >> drivers/media/v4l2-core/videobuf2-dma-contig.c: In function > >> ‘vb2_dc_get_base_sgt’: > >> drivers/media/v4l2-core/videobuf2-dma-contig.c:385: > >> error: implicit declaration of function ‘dma_get_sgtable’ make[7]: *** > >> [drivers/media/v4l2-core/videobuf2-dma-contig.o] Error 1 > >> > >> VIDEO_RENESAS_VSP1 (which doesn't have a platform dependency) selects > >> VIDEOBUF2_DMA_CONTIG, but the latter depends on HAS_DMA. > >> > >> Make VIDEO_RENESAS_VSP1 depend on HAS_DMA to fix this. > > > > Is there a chance we could fix the Kconfig infrastructure instead ? It > > warns about the unmet dependency, shouldn't it disallow selecting the > > driver in the first place ? I have a vague feeling that this topic has > > been discussed before though. > > This has come up several times before. > Unfortunately "select" was "designed" to circumvent all dependencies of > the target symbol. I suppose that fixing this "design bug" (or feature, depending on how one sees it) has been discussed extensively in the past and that the behaviour will not change in the near future. I'll take your patch in and push it to v3.12. > > If that's not possible, > > > >> Signed-off-by: Geert Uytterhoeven > > > > Acked-by: Laurent Pinchart -- Regards, Laurent Pinchart -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v4 0/3] cleanup of gpio_pcf857x.c
Hi Linus, On Tuesday 17 September 2013 14:51:48 Linus Walleij wrote: > On Wed, Sep 4, 2013 at 9:03 AM, George Cherian wrote: > > This patch series > > > > - removes the irq_demux_work > > - Uses devm_request_threaded_irq > > - Call the user handler iff gpio_to_irq is done. > > > > v1 --> v2 > > > > Split v1 to 3 patches > > > > v2 --> v3 > > > > Remove the unnecessary dts patches. > > > > v3 --> v4 > > > > Remove gpio->irq (in patch 2) > > Applied all three patches with Kuninoro's ACK. > > However your last iteration was still labeled "v3" in the subject > so I was a bit confused, but I took all those sent on sep 4. > > Please check the result in my GPIO tree or linux-next and > make sure it works. > > > Note: these patches were made after applying [1]. > > [1] - [PATCH v5] gpio: pcf857x: Add OF support - > > https://lkml.org/lkml/2013/8/27/70 > > Well that patch is not finished, but by rebasing patch 2 with > patch -p1 < patch2.patch it applied anyway. > > Now Laurent has to finalize his DT bindings on top of your patches instead. I will do so. At first sight I don't see any conflict issue. -- Regards, Laurent Pinchart -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: PCF857x and 16-bit GPIO expanders
On Wednesday 18 September 2013 13:16:27 Linus Walleij wrote: > On Tue, Sep 17, 2013 at 9:07 PM, Felipe Balbi wrote: > > has anyone ever successfully using gpio-pcf857x.c driver with 16-bit > > gpio expanders ? We're having some issues here where toggling the last > > gpio pin (gpio 15) on a PCF8575 device causes platform to hang and I > > can't come up with any explanation of why would it hang... > > Bouncing the question to George, Laurent and Kuninori... I've got a board with a PCF8575 chip, but it uses I/Os 8 to 14 only as far as I know. I can try toggling I/O 15, but that will need to wait until next week as I'm currently travelling without access to the hardware. -- Regards, Laurent Pinchart -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] [media] media/v4l2: VIDEO_RENESAS_VSP1 should depend on HAS_DMA
Hi Geert, On Friday 06 September 2013 14:43:56 Geert Uytterhoeven wrote: > If NO_DMA=y: > > warning: (... && VIDEO_RENESAS_VSP1 && ...) selects VIDEOBUF2_DMA_CONTIG > which has unmet direct dependencies (MEDIA_SUPPORT && HAS_DMA) > > drivers/media/v4l2-core/videobuf2-dma-contig.c: In function ‘vb2_dc_mmap’: > drivers/media/v4l2-core/videobuf2-dma-contig.c:202: error: implicit > declaration of function ‘dma_mmap_coherent’ > drivers/media/v4l2-core/videobuf2-dma-contig.c: In function > ‘vb2_dc_get_base_sgt’: drivers/media/v4l2-core/videobuf2-dma-contig.c:385: > error: implicit declaration of function ‘dma_get_sgtable’ make[7]: *** > [drivers/media/v4l2-core/videobuf2-dma-contig.o] Error 1 > > VIDEO_RENESAS_VSP1 (which doesn't have a platform dependency) selects > VIDEOBUF2_DMA_CONTIG, but the latter depends on HAS_DMA. > > Make VIDEO_RENESAS_VSP1 depend on HAS_DMA to fix this. Is there a chance we could fix the Kconfig infrastructure instead ? It warns about the unmet dependency, shouldn't it disallow selecting the driver in the first place ? I have a vague feeling that this topic has been discussed before though. If that's not possible, > Signed-off-by: Geert Uytterhoeven Acked-by: Laurent Pinchart > --- > drivers/media/platform/Kconfig |2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/media/platform/Kconfig b/drivers/media/platform/Kconfig > index 8068d7b..fbc0611 100644 > --- a/drivers/media/platform/Kconfig > +++ b/drivers/media/platform/Kconfig > @@ -212,7 +212,7 @@ config VIDEO_SH_VEU > > config VIDEO_RENESAS_VSP1 > tristate "Renesas VSP1 Video Processing Engine" > - depends on VIDEO_V4L2 && VIDEO_V4L2_SUBDEV_API > + depends on VIDEO_V4L2 && VIDEO_V4L2_SUBDEV_API && HAS_DMA > select VIDEOBUF2_DMA_CONTIG > ---help--- > This is a V4L2 driver for the Renesas VSP1 video processing engine. -- Regards, Laurent Pinchart -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] RFC: interrupt consistency check for OF GPIO IRQs
Hi Linus, Sorry for the late reply. On Thursday 22 August 2013 00:02:39 Linus Walleij wrote: > On Tue, Aug 20, 2013 at 12:04 AM, Laurent Pinchart wrote: > > On Wednesday 31 July 2013 01:44:53 Linus Walleij wrote: > >> I don't see how sharing works here, or how another user, i.e. another one > >> than the user wanting to recieve the IRQ, can validly request such a > >> line? What would the usecase for that valid request be? > > > > When the GPIO is wired to a status signal (such as an MMC card detect > > signal) the driver might want to read the state of the signal > > independently of the interrupt handler. > > That is true. But for such a complex usecase I think it's reasonable that > we only specify the GPIO in the device tree, and the driver utilizing the > IRQ need to take that and perform gpio_to_irq() on it, and then it still > works to use it both ways. I'm pretty sure I would have had an objection a couple of weeks ago when I was looking into this, but I can't think of another use case for now, so I agree with you. > >> Basically I believe these two things need to be exclusive in the DT > >> world: > >> > >> A: request_irq(a resource passed from "interrupts"); > >> -> core implicitly performs gpio_request() > >> gpio_direction_input() > >> > >> B: gpio_request(a resource passed from "gpios"); > >> gpio_direction_input() > >> request_irq(gpio_to_irq()) > >> > >> Never both. And IIUC that was what happened in the OMAP case. > > > > Isn't the core issue that we can translate a GPIO number to an IRQ number, > > but not the other way around ? If that could be done, we could request > > the GPIO and configure it as an input when the IRQ is requested. > > That is true. It would be easier if all GPIO drivers has an irqchip and > and irqdomain, then we could implement irq_to_gpio() properly in gpiolib > and this would not be a problem. Alas, not all do. > > But I also think that the DT contains (as demonstrated by the patch) > all information about what interrupts and GPIOs may conflict, so I > also see this as something of a consistency check, but it could go > in either way. -- Regards, Laurent Pinchart -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] SHMOBILE: DRM: Fix backlight_device register and unregister undefined errors.
Hi Majunath, Thank you for the patch. On Tuesday 08 October 2013 16:09:17 Majunath Goudar wrote: > This patch adds a BACKLIGHT_CLASS_DEVICE dependency to configure the > DRM_SHMOBILE. Without this patch, build system can lead to build failure. > This was observed during randconfig testing, in which DRM_SHMOBILE was > enabled w/o BACKLIGHT_CLASS_DEVICE being enabled. Following was the error: > > Building modules, stage 2. > MODPOST 516 modules > ERROR: "backlight_device_register" [drivers/gpu/drm/shmobile/shmob-drm.ko] > undefined! ERROR: "backlight_device_unregister" > [drivers/gpu/drm/shmobile/shmob-drm.ko] undefined! make[1]: *** [__modpost] > Error 1 > make: *** [modules] Error 2 > > Signed-off-by: Manjunath Goudar > Cc: David Airlie > Cc: Laurent Pinchart > Cc: Sascha Hauer > Cc: dri-de...@lists.freedesktop.org > Cc: linux-kernel@vger.kernel.org > --- > drivers/gpu/drm/shmobile/Kconfig |2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/shmobile/Kconfig > b/drivers/gpu/drm/shmobile/Kconfig index ca498d1..eaf822e 100644 > --- a/drivers/gpu/drm/shmobile/Kconfig > +++ b/drivers/gpu/drm/shmobile/Kconfig > @@ -1,6 +1,6 @@ > config DRM_SHMOBILE > tristate "DRM Support for SH Mobile" > - depends on DRM && (ARM || SUPERH) > + depends on DRM && (ARM || SUPERH) && BACKLIGHT_CLASS_DEVICE What about select BACKLIGHT_CLASS_DEVICE instead ? That's what the i915, radeon, gma500, tilcdc and nouveau drivers do. > select DRM_KMS_HELPER > select DRM_KMS_CMA_HELPER > select DRM_GEM_CMA_HELPER -- Regards, Laurent Pinchart -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v5] gpio: pcf857x: Add OF support
Hi Linus, On Thursday 29 August 2013 20:24:32 Linus Walleij wrote: > On Wed, Aug 28, 2013 at 1:58 PM, Laurent Pinchart wrote: > > On Tuesday 27 August 2013 19:18:55 Wolfram Sang wrote: > >> > The driver should support the same chip models reardless of whether > >> > it's used with or without DT. If an entry in the OF table has no > >> > corresponding entry in the I2C table I would consider that as a driver > >> > bug. > >> > >> Linus Walleij posted a patch to support DT only probing, but too many > >> side-effects showed up. Some were fixable (probe regressions) and some > >> need more work, if feasible at all. Most prominent is that runtime > >> instantiation of i2c slaves currently needs an entry in the i2c table. > > > > Linus, I'd like to get this in v3.12 if it's not too late. Could you > > please provide your input ? > > Provided some input on the v4 version, Do you mean v4 of this patch ? I can't find your reply. > due to being a bit overloaded my patch queue is overloaded... No worries. We can delay this one until v3.13. -- Regards, Laurent Pinchart -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v5] gpio: pcf857x: Add OF support
On Friday 30 August 2013 01:44:50 Laurent Pinchart wrote: > On Thursday 29 August 2013 20:24:32 Linus Walleij wrote: > > On Wed, Aug 28, 2013 at 1:58 PM, Laurent Pinchart wrote: > > > On Tuesday 27 August 2013 19:18:55 Wolfram Sang wrote: > > >> > The driver should support the same chip models reardless of whether > > >> > it's used with or without DT. If an entry in the OF table has no > > >> > corresponding entry in the I2C table I would consider that as a > > >> > driver > > >> > bug. > > >> > > >> Linus Walleij posted a patch to support DT only probing, but too many > > >> side-effects showed up. Some were fixable (probe regressions) and some > > >> need more work, if feasible at all. Most prominent is that runtime > > >> instantiation of i2c slaves currently needs an entry in the i2c table. > > > > > > Linus, I'd like to get this in v3.12 if it's not too late. Could you > > > please provide your input ? > > > > Provided some input on the v4 version, > > Do you mean v4 of this patch ? I can't find your reply. Scratch that, I've found it. Sorry for the noise. > > due to being a bit overloaded my patch queue is overloaded... > > No worries. We can delay this one until v3.13. -- Regards, Laurent Pinchart -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v4] gpio: pcf857x: Add OF support
Hi Linus, On Thursday 29 August 2013 14:16:59 Linus Walleij wrote: > On Mon, Aug 26, 2013 at 3:45 PM, Laurent Pinchart wrote: > > Add DT bindings for the pcf857x-compatible chips and parse the device > > tree node in the driver. > > > > Signed-off-by: Laurent Pinchart > > > > First: can I get an ACK from some DT-bindings maintainer? > > I think you may need to CC them all individually to get some response. I'll make sure to get an ACK for v6 (or a more recent version). Reviewers are probably busy with the merge window about to open, delaying this patch to v3.13 should help. I'll post v6 after receiving your answer to the comment below, as well as to the issue raised by Tomasz on v5. > > +++ b/Documentation/devicetree/bindings/gpio/gpio-pcf857x.txt > > @@ -0,0 +1,71 @@ > > +* PCF857x-compatible I/O expanders > > + > > +The PCF857x-compatible chips have "quasi-bidirectional" I/O pins that can > > be +driven high by a pull-up current source or driven low to ground. This > > combines +the direction and output level into a single bit per pin, which > > can't be read +back. We can't actually know at initialization time > > whether a pin is configured +(a) as output and driving the signal > > low/high, or (b) as input and reporting a +low/high value, without > > knowing the last value written since the chip came out +of reset (if > > any). The only reliable solution for setting up pin direction is +thus to > > do it explicitly. > > Nitpick: I prefer that wrt gpio we talk about "lines" rather than "pins" > to separate it from the pin control concept. Just > s/pin/line/g OK. > (...) > > > +Optional Properties: > > + > > + - pins-initial-state: Bitmask that specifies the initial state of each > > pin. + When a bit is set to zero, the corresponding pin will be > > initialized to the + input (pulled-up) state. When the bit is set to > > one, the pin will be + initialized the the low-level output state. If > > the property is not specified + all pins will be initialized to the > > input state. > > Name this lines-initial-states (pluralis). OK. > Don't we want to do this generic if we shall do it? > > Like for *any* GPIO chips we provide lines-initial state in the device > tree and some code in the gpiochip with a callback in struct gpio_chip > that can be called by the gpiolib core to set this up? Then we don't > have to reimplement this for every GPIO controller that needs it. Most GPIO chips will provide a way to read back the current state. The initial state only needs to be provided for write-only chips. This is (luckily) rather an exception, so I don't think we should implement it in the core, at least not yet. We can always refactor the code later if needed, the proposed DT binding is generic enough. > Sorry for not noticing this earlier... No worries. -- Regards, Laurent Pinchart -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/1] [media] uvcvideo: quirk PROBE_DEF for Dell SP2008WFP monitor.
Hi Joseph,
Thank you for the patch.
On Thursday 29 August 2013 11:17:41 Joseph Salisbury wrote:
> BugLink: http://bugs.launchpad.net/bugs/1217957
>
> Add quirk for Dell SP2008WFP monitor: 05a9:2641
>
> Signed-off-by: Joseph Salisbury
> Tested-by: Christopher Townsend
> Cc: Laurent Pinchart
> Cc: Mauro Carvalho Chehab
> Cc: linux-me...@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
> Cc: sta...@vger.kernel.org
Acked-by: Laurent Pinchart
I've applied it to my tree. Given that we're too close to the v3.12 merge
window I will push it for v3.13.
> ---
> drivers/media/usb/uvc/uvc_driver.c |9 +
> 1 file changed, 9 insertions(+)
>
> diff --git a/drivers/media/usb/uvc/uvc_driver.c
> b/drivers/media/usb/uvc/uvc_driver.c index ed123f4..8c1826c 100644
> --- a/drivers/media/usb/uvc/uvc_driver.c
> +++ b/drivers/media/usb/uvc/uvc_driver.c
> @@ -2174,6 +2174,15 @@ static struct usb_device_id uvc_ids[] = {
> .bInterfaceSubClass = 1,
> .bInterfaceProtocol = 0,
> .driver_info = UVC_QUIRK_PROBE_DEF },
> + /* Dell SP2008WFP Monitor */
> + { .match_flags = USB_DEVICE_ID_MATCH_DEVICE
> + | USB_DEVICE_ID_MATCH_INT_INFO,
> + .idVendor = 0x05a9,
> + .idProduct= 0x2641,
> + .bInterfaceClass = USB_CLASS_VIDEO,
> + .bInterfaceSubClass = 1,
> + .bInterfaceProtocol = 0,
> + .driver_info = UVC_QUIRK_PROBE_DEF },
> /* Dell Alienware X51 */
> { .match_flags = USB_DEVICE_ID_MATCH_DEVICE
>
> | USB_DEVICE_ID_MATCH_INT_INFO,
--
Regards,
Laurent Pinchart
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v4] gpio: pcf857x: Add OF support
On Friday 30 August 2013 10:07:23 Linus Walleij wrote: > On Fri, Aug 30, 2013 at 2:05 AM, Laurent Pinchart wrote: > > On Thursday 29 August 2013 14:16:59 Linus Walleij wrote: > >> On Mon, Aug 26, 2013 at 3:45 PM, Laurent Pinchart wrote: > >> > >> Don't we want to do this generic if we shall do it? > >> > >> Like for *any* GPIO chips we provide lines-initial state in the device > >> tree and some code in the gpiochip with a callback in struct gpio_chip > >> that can be called by the gpiolib core to set this up? Then we don't > >> have to reimplement this for every GPIO controller that needs it. > > > > Most GPIO chips will provide a way to read back the current state. The > > initial state only needs to be provided for write-only chips. This is > > (luckily) rather an exception, so I don't think we should implement it in > > the core, at least not yet. We can always refactor the code later if > > needed, the proposed DT binding is generic enough. > > But I think this can be useful on any GPIO chip. > > For someone deploying some system and hacking around in the > device tree to set the GPIOs up properly at boot it can be a > real useful tool. > > Or is that giving them too much rope? :-D That's the job of the boot loader, isn't it ? :-) -- Regards, Laurent Pinchart -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/1] [media] uvcvideo: quirk PROBE_DEF for Dell SP2008WFP monitor.
Hi Greg, On Thursday 29 August 2013 21:00:21 Greg KH wrote: > On Fri, Aug 30, 2013 at 02:41:17AM +0200, Laurent Pinchart wrote: > > On Thursday 29 August 2013 11:17:41 Joseph Salisbury wrote: > > > BugLink: http://bugs.launchpad.net/bugs/1217957 > > > > > > Add quirk for Dell SP2008WFP monitor: 05a9:2641 > > > > > > Signed-off-by: Joseph Salisbury > > > Tested-by: Christopher Townsend > > > Cc: Laurent Pinchart > > > Cc: Mauro Carvalho Chehab > > > Cc: linux-me...@vger.kernel.org > > > Cc: linux-kernel@vger.kernel.org > > > Cc: sta...@vger.kernel.org > > > > Acked-by: Laurent Pinchart > > > > I've applied it to my tree. Given that we're too close to the v3.12 merge > > window I will push it for v3.13. > > A quirk has to wait that long? That's not ok, they should go in much > sooner than that... Can such a patch get merged during the -rc phase ? If so I will push it to v3.12. -- Regards, Laurent Pinchart -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/1] [media] uvcvideo: quirk PROBE_DEF for Dell SP2008WFP monitor.
Hi Greg, On Friday 30 August 2013 09:39:58 Greg KH wrote: > On Fri, Aug 30, 2013 at 12:28:16PM +0200, Laurent Pinchart wrote: > > On Thursday 29 August 2013 21:00:21 Greg KH wrote: > > > On Fri, Aug 30, 2013 at 02:41:17AM +0200, Laurent Pinchart wrote: > > > > On Thursday 29 August 2013 11:17:41 Joseph Salisbury wrote: > > > > > BugLink: http://bugs.launchpad.net/bugs/1217957 > > > > > > > > > > Add quirk for Dell SP2008WFP monitor: 05a9:2641 > > > > > > > > > > Signed-off-by: Joseph Salisbury > > > > > Tested-by: Christopher Townsend > > > > > Cc: Laurent Pinchart > > > > > Cc: Mauro Carvalho Chehab > > > > > Cc: linux-me...@vger.kernel.org > > > > > Cc: linux-kernel@vger.kernel.org > > > > > Cc: sta...@vger.kernel.org > > > > > > > > Acked-by: Laurent Pinchart > > > > > > > > I've applied it to my tree. Given that we're too close to the v3.12 > > > > merge window I will push it for v3.13. > > > > > > A quirk has to wait that long? That's not ok, they should go in much > > > sooner than that... > > > > Can such a patch get merged during the -rc phase ? If so I will push it to > > v3.12. > > Yes it can, OK, I'll send a pull request to Mauro right after the v3.12 merge window closes, as he's pretty busy with pending pull requests for v3.12 at the moment. > and it should also be merged to stable releases, as the cc: stable shows. Sure, that was my plan. -- Regards, Laurent Pinchart -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/4] pinctrl: sh-pfc: r8a7790: add pin definitions for the I2C3 interface
Hi Guennadi,
On Thursday 26 September 2013 11:24:26 Guennadi Liakhovetski wrote:
> On Wed, 25 Sep 2013, Laurent Pinchart wrote:
> > On Monday 09 September 2013 18:03:53 Guennadi Liakhovetski wrote:
> > > There are four I2C interfaces on r8a7790, each of them can be connected
> > > to one of the two respective I2C controllers, e.g. interface #0 can be
> > > configured to work with I2C0 or with IIC0. Additionally some of those
> > > interfaces can also use one of several pin sets. Interface #3 is
> > > special, because it can be used in automatic mode for DVFS. It only has
> > > one set of pins available and those pins cannot be used for anything
> > > else, they also lack the GPIO function.
> > >
> > > This patch uses the sh-pfc ability to configure pins, not associated
> > > with GPIOs and adds support for I2C3 to the r8a7790 PFC set up.
> >
> > Ulrich Hecht sent a patch titled "sh-pfc: r8a7790: Add I2C pin groups and
> > functions" that added pin groups for I2C1 and I2C2. The patch is available
> > from
> >
> > git://linuxtv.org/pinchartl/fbdev.git pinmux/next
> >
> > If you need to resubmit this patch due to my comments below, could you
> > please rebase it on top of that branch ?
> >
> > > Signed-off-by: Guennadi Liakhovetski
> > > ---
> > >
> > > drivers/pinctrl/sh-pfc/pfc-r8a7790.c | 28
> > >
> > > 1 files changed, 28 insertions(+), 0 deletions(-)
> > >
> > > diff --git a/drivers/pinctrl/sh-pfc/pfc-r8a7790.c
> > > b/drivers/pinctrl/sh-pfc/pfc-r8a7790.c index 64fcc006..c3c4d9b 100644
> > > --- a/drivers/pinctrl/sh-pfc/pfc-r8a7790.c
> > > +++ b/drivers/pinctrl/sh-pfc/pfc-r8a7790.c
> > > @@ -781,6 +781,8 @@ enum {
> > > ADICS_SAMP_MARK, DU2_CDE_MARK, QPOLB_MARK, SCIFA2_RXD_B_MARK,
> > > USB1_PWEN_MARK, AUDIO_CLKOUT_D_MARK, USB1_OVC_MARK,
> > > TCLK1_B_MARK,
> > > +
> > > + I2C3_SCL_MARK, I2C3_SDA_MARK,
> > >
> > > PINMUX_MARK_END,
> > > };
> > > @@ -1719,10 +1721,22 @@ static const u16 pinmux_data[] = {
> > > PINMUX_IPSR_DATA(IP16_6, AUDIO_CLKOUT_D),
> > > PINMUX_IPSR_DATA(IP16_7, USB1_OVC),
> > > PINMUX_IPSR_MODSEL_DATA(IP16_7, TCLK1_B, SEL_TMU1_1),
> > > +
> > > + PINMUX_DATA(I2C3_SCL_MARK, FN_SEL_IICDVFS_1),
> > > + PINMUX_DATA(I2C3_SDA_MARK, FN_SEL_IICDVFS_1),
> >
> > You introduce a way to mux the I2C3 function on those two pins, but no way
> > to select the IICDVFS back. I don't think it's an issue, we can always
> > add that later when (if) needed. Linus, is that fine with you ?
>
> I did it on purpose, since I didn't have a use case for IICDVFS. I prefer
> not to add too many things, that cannot be tested.
Just for the record, I'm fine with that.
> > > };
> > >
> > > +/* R8A7790 has 6 banks with 32 GPIOs in each = 192 GPIOs */
> > > +#define ROW_GROUP_A(r) ('Z' - 'A' + 1 + (r))
> > > +#define PIN_NUMBER(r, c) (((r) - 'A') * 16 + (c) + 200)
> >
> > The BGA package has 31 columns, shouldn't you multiply the row number by
> > 31 instead of 16 ?
>
> Oops, you're right - the pin table is continued on the second page...
Could you then submit a v2 based on git://linuxtv.org/pinchartl/fbdev.git
pinmux/next ?
> > As we have 192 GPIOs, shouldn't you use an offset of 192 instead of 200 ?
> > This doesn't matter too much I guess.
>
> On one of Renesas SoCs I've seen an offset of 2000 used. I thought it
> would be an exaggeration in this case ;) but I followed the pattern of
> using a round number for the offset, is this ok?
I suppose that's fine, yes.
--
Regards,
Laurent Pinchart
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 5/5] ARM: shmobile: lager: (DEVEL) add CPUFreq support
Hi Guennadi,
Thank you for the patch.
On Thursday 26 September 2013 19:21:00 Guennadi Liakhovetski wrote:
> The Lager board uses a DA9210 voltage regulator to supply DVFS power to the
> CA15 cores on the r8a7790 SoC. This patch adds CPUFreq support for that
> board using the cpufreq-cpu0 driver.
>
> Signed-off-by: Guennadi Liakhovetski
> ---
>
> v2: added 'status = "okay";' to the i2c bus
>
> arch/arm/boot/dts/r8a7790-lager-reference.dts | 33 +
> arch/arm/mach-shmobile/board-lager-reference.c |4 ++-
> 2 files changed, 36 insertions(+), 1 deletions(-)
>
> diff --git a/arch/arm/boot/dts/r8a7790-lager-reference.dts
> b/arch/arm/boot/dts/r8a7790-lager-reference.dts index c462ef1..1ce0a97
> 100644
> --- a/arch/arm/boot/dts/r8a7790-lager-reference.dts
> +++ b/arch/arm/boot/dts/r8a7790-lager-reference.dts
> @@ -43,3 +43,36 @@
> };
> };
> };
> +
> +&i2c3 {
> + status = "okay";
> + pinctrl-names = "default";
> + pinctrl-0 = <&i2c3_pins>;
> +
> + vdd_dvfs: da9210@68 {
> + compatible = "diasemi,da9210";
> + reg = <0x68>;
> +
> + regulator-min-microvolt = <90>;
> + regulator-max-microvolt = <100>;
> + regulator-boot-on;
> + regulator-always-on;
> + };
> +};
> +
> +&cpu0 {
> + cpu0-supply = <&vdd_dvfs>;
> + operating-points = <
> + /* kHz uV - OPs unknown yet */
> + 130 100
> + 100 90
> + >;
> + voltage-tolerance = <1>; /* 1% */
> +};
> +
> +&pfc {
> + i2c3_pins: i2c3 {
> + renesas,groups = "i2c3";
> + renesas,function = "i2c3";
> + };
> +};
> diff --git a/arch/arm/mach-shmobile/board-lager-reference.c
> b/arch/arm/mach-shmobile/board-lager-reference.c index 1a1a4a8..2bc8bae
> 100644
> --- a/arch/arm/mach-shmobile/board-lager-reference.c
> +++ b/arch/arm/mach-shmobile/board-lager-reference.c
> @@ -20,6 +20,7 @@
>
> #include
> #include
> +#include
> #include
> #include
>
> @@ -29,7 +30,8 @@ static void __init lager_add_standard_devices(void)
> r8a7790_clock_init();
>
> r8a7790_add_dt_devices();
> -of_platform_populate(NULL, of_default_bus_match_table, NULL, NULL);
> + of_platform_populate(NULL, of_default_bus_match_table, NULL, NULL);
> + platform_device_register_simple("cpufreq-cpu0", -1, NULL, 0);
Out of curiosity, could you explain how this cpufreq-cpu0 platform device gets
associated with the cpu0 DT node ? The cpufreq-cpu0 driver requires a DT node
(its probe function returns -ENOENT if pdev->dev.of_node is NULL), and I don't
see how the of_node gets set as the platform device is registered through
board code. I might of course be missing something obvious :-)
> }
>
> static const char *lager_boards_compat_dt[] __initdata = {
--
Regards,
Laurent Pinchart
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 1/5] pinctrl: sh-pfc: r8a7790: add pin definitions for the I2C3 interface
Hi Guennadi,
Thank you for the patch.
On Thursday 26 September 2013 19:20:56 Guennadi Liakhovetski wrote:
> There are four I2C interfaces on r8a7790, each of them can be connected to
> one of the two respective I2C controllers, e.g. interface #0 can be
> configured to work with I2C0 or with IIC0. Additionally some of those
> interfaces can also use one of several pin sets. Interface #3 is special,
> because it can be used in automatic mode for DVFS. It only has one set
> of pins available and those pins cannot be used for anything else, they
> also lack the GPIO function.
>
> This patch uses the sh-pfc ability to configure pins, not associated with
> GPIOs and adds support for I2C3 to the r8a7790 PFC set up.
>
> Signed-off-by: Guennadi Liakhovetski
Acked-by: Laurent Pinchart
I've taken the patch in my tree and sent an update to my previous pull request
for the pinctrl tree.
> ---
>
> v2: s/16/31/ in pin number calculation, thanks to Laurent for pointing out.
>
> drivers/pinctrl/sh-pfc/pfc-r8a7790.c | 28
> 1 files changed, 28 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/pinctrl/sh-pfc/pfc-r8a7790.c
> b/drivers/pinctrl/sh-pfc/pfc-r8a7790.c index 5c2657b..72786fc 100644
> --- a/drivers/pinctrl/sh-pfc/pfc-r8a7790.c
> +++ b/drivers/pinctrl/sh-pfc/pfc-r8a7790.c
> @@ -781,6 +781,8 @@ enum {
> ADICS_SAMP_MARK, DU2_CDE_MARK, QPOLB_MARK, SCIFA2_RXD_B_MARK,
> USB1_PWEN_MARK, AUDIO_CLKOUT_D_MARK, USB1_OVC_MARK,
> TCLK1_B_MARK,
> +
> + I2C3_SCL_MARK, I2C3_SDA_MARK,
> PINMUX_MARK_END,
> };
>
> @@ -1719,10 +1721,22 @@ static const u16 pinmux_data[] = {
> PINMUX_IPSR_DATA(IP16_6, AUDIO_CLKOUT_D),
> PINMUX_IPSR_DATA(IP16_7, USB1_OVC),
> PINMUX_IPSR_MODSEL_DATA(IP16_7, TCLK1_B, SEL_TMU1_1),
> +
> + PINMUX_DATA(I2C3_SCL_MARK, FN_SEL_IICDVFS_1),
> + PINMUX_DATA(I2C3_SDA_MARK, FN_SEL_IICDVFS_1),
> };
>
> +/* R8A7790 has 6 banks with 32 GPIOs in each = 192 GPIOs */
> +#define ROW_GROUP_A(r) ('Z' - 'A' + 1 + (r))
> +#define PIN_NUMBER(r, c) (((r) - 'A') * 31 + (c) + 200)
> +#define PIN_A_NUMBER(r, c) PIN_NUMBER(ROW_GROUP_A(r), c)
> +
> static struct sh_pfc_pin pinmux_pins[] = {
> PINMUX_GPIO_GP_ALL(),
> +
> + /* Pins not associated with a GPIO port */
> + SH_PFC_PIN_NAMED(ROW_GROUP_A('J'), 15, AJ15),
> + SH_PFC_PIN_NAMED(ROW_GROUP_A('H'), 15, AH15),
> };
>
> /* - DU RGB
> - */ @@
> -2048,6 +2062,14 @@ static const unsigned int i2c2_e_pins[] = { static
> const unsigned int i2c2_e_mux[] = {
> I2C2_SCL_E_MARK, I2C2_SDA_E_MARK,
> };
> +/* - I2C3
> --- */
> +static const unsigned int i2c3_pins[] = {
> + /* SCL, SDA */
> + PIN_A_NUMBER('J', 15), PIN_A_NUMBER('H', 15),
> +};
> +static const unsigned int i2c3_mux[] = {
> + I2C3_SCL_MARK, I2C3_SDA_MARK,
> +};
> /* - INTC
> --- */
> static const unsigned int intc_irq0_pins[] = {
> /* IRQ */
> @@ -3113,6 +3135,7 @@ static const struct sh_pfc_pin_group pinmux_groups[] =
> { SH_PFC_PIN_GROUP(i2c2_c),
> SH_PFC_PIN_GROUP(i2c2_d),
> SH_PFC_PIN_GROUP(i2c2_e),
> + SH_PFC_PIN_GROUP(i2c3),
> SH_PFC_PIN_GROUP(intc_irq0),
> SH_PFC_PIN_GROUP(intc_irq1),
> SH_PFC_PIN_GROUP(intc_irq2),
> @@ -3323,6 +3346,10 @@ static const char * const i2c2_groups[] = {
> "i2c2_e",
> };
>
> +static const char * const i2c3_groups[] = {
> + "i2c3",
> +};
> +
> static const char * const intc_groups[] = {
> "intc_irq0",
> "intc_irq1",
> @@ -3551,6 +3578,7 @@ static const struct sh_pfc_function pinmux_functions[]
> = { SH_PFC_FUNCTION(hscif1),
> SH_PFC_FUNCTION(i2c1),
> SH_PFC_FUNCTION(i2c2),
> + SH_PFC_FUNCTION(i2c3),
> SH_PFC_FUNCTION(intc),
> SH_PFC_FUNCTION(mmc0),
> SH_PFC_FUNCTION(mmc1),
--
Regards,
Laurent Pinchart
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] [media] uvcvideo: fix data type for pan/tilt control
Hi Chanho,
Thank you for the patch.
On Friday 27 September 2013 13:57:40 Chanho Min wrote:
> The pan/tilt absolute control value is signed value. If minimum value
> is minus, It will be changed to plus by clamp_t() as commit 64ae9958a62.
> ([media] uvcvideo: Fix control value clamping for unsigned integer
> controls).
>
> It leads to wrong setting of the control values. For example,
> when min and max are -36000 and 36000, the setting value between of this
> range is always 36000. So, Its data type should be changed to signed.
>
> Signed-off-by: Chanho Min
Acked-by: Laurent Pinchart
I've taken the patch in my tree and will send a pull request for the next
kernel version.
> ---
> drivers/media/usb/uvc/uvc_ctrl.c |4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/media/usb/uvc/uvc_ctrl.c
> b/drivers/media/usb/uvc/uvc_ctrl.c index a2f4501..0eb82106 100644
> --- a/drivers/media/usb/uvc/uvc_ctrl.c
> +++ b/drivers/media/usb/uvc/uvc_ctrl.c
> @@ -664,7 +664,7 @@ static struct uvc_control_mapping uvc_ctrl_mappings[] =
> { .size = 32,
> .offset = 0,
> .v4l2_type = V4L2_CTRL_TYPE_INTEGER,
> - .data_type = UVC_CTRL_DATA_TYPE_UNSIGNED,
> + .data_type = UVC_CTRL_DATA_TYPE_SIGNED,
> },
> {
> .id = V4L2_CID_TILT_ABSOLUTE,
> @@ -674,7 +674,7 @@ static struct uvc_control_mapping uvc_ctrl_mappings[] =
> { .size = 32,
> .offset = 32,
> .v4l2_type = V4L2_CTRL_TYPE_INTEGER,
> - .data_type = UVC_CTRL_DATA_TYPE_UNSIGNED,
> + .data_type = UVC_CTRL_DATA_TYPE_SIGNED,
> },
> {
> .id = V4L2_CID_PRIVACY,
--
Regards,
Laurent Pinchart
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2] pwm-backlight: allow for non-increasing brightness levels
Hi Jingoo, On Friday 27 September 2013 12:28:21 Jingoo Han wrote: > On Thursday, September 26, 2013 9:08 PM, Tomi Valkeinen wrote: > > On 26/09/13 14:51, Thierry Reding wrote: > > > On Thu, Sep 26, 2013 at 01:03:06PM +0300, Tomi Valkeinen wrote: > > > [...] > > > > > >> But if you and Thierry think this version is good, I'll take it. > > > > > > That sounds like you want to take it through the fbdev tree. Jingoo is > > > listed (along with Richard, but he hasn't been responsive to email for > > > years) as maintainer for the backlight subsystem. Furthermore back at > > > > Ah, so they are. I just thought it falls under fbdev, as it's under > > drivers/video/ =). > > > > I don't have any particular "want" to take it through fbdev tree. But I > > can take it. > > > > > the time when I began working on the PWM subsystem, the backlight sub- > > > system was pretty much orphaned, and pwm-backlight was by far the > > > biggest user of the PWM subsystem. I adopted the driver at the time > > > because it needed to be updated for PWM subsystem changes. > > > > > > What's the plan going forward? Given the coupling between the PWM > > > subsystem and the pwm-backlight driver it might be useful to keep > > > maintaining it as part of the PWM subsystem. On the other hand, there's > > > some coupling between the driver and the backlight subsystem too. > > > > And backlight is coupled with fbdev... Which is something I don't like. > > +cc Laurent Pinchart, > > Yes, right. > The backlight should be de-coupled with fbdev. > I remember that Laurent Pinchart was doing this patch. > > Laurent Pinchart, > Would you let us know your plan about this? :-) My plans include finishing CDF first :-) I thus don't know when I'll have time to tackle this task. Feel free to pick it but. If you do, I would appreciate if you could discuss your ideas with me. > > > I have a couple of patches queued up for 3.13 that rework parts of the > > > driver, so it'd be good to know how you guys want to handle this. > > > > Well. I'm happy if somebody wants to maintain the backlight side. In > > fact, I'd be happy if somebody would start restructuring it totally, > > it's rather messy. The link with fbdev should be removed, and some > > backlight drivers are actually panel drivers. However, perhaps Common > > Display Framework is required until it can be fully cleaned. > > I think that some backlight drivers can be moved to 'Common Display > Framework', after 'Common Display Framework' is merged. > But, I am not sure, when it will be completed. Hardware backlight devices don't process video streams, so they don't really belong to CDF, at least to the CDF that we know today. However, a hardware panel device that integrates a backlight would be supported by a CDF driver, which would create a Linux backlight device. > > So... For the time being, I'm fine with merging pwm-backlight via any > > tree that works best. I'm presuming here that backlight framework and > > fbdev (for the parts that are relevant for backlight) are not really > > being changed, so there shouldn't be conflicts. -- Regards, Laurent Pinchart -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Revert "V4L/DVB: uvc: Enable USB autosuspend by default on uvcvideo"
Hi, On Tuesday 24 September 2013 08:55:19 VDR User wrote: > On Tue, Sep 24, 2013 at 4:34 AM, Laurent Pinchart 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. It's not just my setup, auto-suspend works for the vast majority of webcams. It has been enabled three years ago, with a report that Fedora had enabled it by carrying a kernel patch for a while, without any user complaint. > I don't see any reasonable argument against letting the user enable it if > he/she wants it. USB autosuspend is an important power saving feature. I would be fine with enabling it in userspace if we could find a reasonable, cross-distro way to create, maintain and distribute the list of devices that support USB autosuspend properly. -- Regards, Laurent Pinchart -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] ARM: shmobile: ape6evm: fix incorrect placement of __initdata tag
Hi Bartlomiej,
Thank you for the patch.
On Monday 30 September 2013 15:07:47 Bartlomiej Zolnierkiewicz wrote:
> __initdata tag should be placed between the variable name and equal
> sign for the variable to be placed in the intended .init.data section.
>
> In this particular case __initdata is incorrect as ape6evm_keys_pdata
> can be used as a platform data for gpio-keys driver which can be
> compiled as module.
A pointer to the ape6evm_keys_pdata structure is passed to the
platform_device_register_data() function, which makes a copy of the structure.
Marking it as __initdata is thus correct.
> Signed-off-by: Bartlomiej Zolnierkiewicz
> Signed-off-by: Kyungmin Park
> ---
> arch/arm/mach-shmobile/board-ape6evm.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/arm/mach-shmobile/board-ape6evm.c
> b/arch/arm/mach-shmobile/board-ape6evm.c index 7627385..8954f55 100644
> --- a/arch/arm/mach-shmobile/board-ape6evm.c
> +++ b/arch/arm/mach-shmobile/board-ape6evm.c
> @@ -86,7 +86,7 @@ static struct gpio_keys_button gpio_buttons[] = {
> GPIO_KEY(KEY_VOLUMEDOWN,329,"S21"),
> };
>
> -static struct __initdata gpio_keys_platform_data ape6evm_keys_pdata = {
> +static struct gpio_keys_platform_data ape6evm_keys_pdata = {
> .buttons= gpio_buttons,
> .nbuttons = ARRAY_SIZE(gpio_buttons),
> };
--
Regards,
Laurent Pinchart
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/6] serial8250-em: convert to clk_prepare/unprepare
Hi Yoshii-san,
Thank you for the patch.
On Tuesday 24 September 2013 13:10:39 takas...@ops.dti.ne.jp wrote:
> From: Shinya Kuribayashi
>
> Add calls to clk_prepare and unprepare so that EMMA Mobile EV2 can
> migrate to the common clock framework.
>
> Signed-off-by: Shinya Kuribayashi
> [takashi.yoshii...@renesas.com: edited for conflicts]
> Signed-off-by: Takashi Yoshii
Acked-by: Laurent Pinchart
> ---
> drivers/tty/serial/8250/8250_em.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/tty/serial/8250/8250_em.c
> b/drivers/tty/serial/8250/8250_em.c index 5f3bba1..d1a9078 100644
> --- a/drivers/tty/serial/8250/8250_em.c
> +++ b/drivers/tty/serial/8250/8250_em.c
> @@ -122,7 +122,7 @@ static int serial8250_em_probe(struct platform_device
> *pdev) up.port.dev = &pdev->dev;
> up.port.private_data = priv;
>
> - clk_enable(priv->sclk);
> + clk_prepare_enable(priv->sclk);
> up.port.uartclk = clk_get_rate(priv->sclk);
>
> up.port.iotype = UPIO_MEM32;
> @@ -134,7 +134,7 @@ static int serial8250_em_probe(struct platform_device
> *pdev) ret = serial8250_register_8250_port(&up);
> if (ret < 0) {
> dev_err(&pdev->dev, "unable to register 8250 port\n");
> - clk_disable(priv->sclk);
> + clk_disable_unprepare(priv->sclk);
> return ret;
> }
>
> @@ -148,7 +148,7 @@ static int serial8250_em_remove(struct platform_device
> *pdev) struct serial8250_em_priv *priv = platform_get_drvdata(pdev);
>
> serial8250_unregister_port(priv->line);
> - clk_disable(priv->sclk);
> + clk_disable_unprepare(priv->sclk);
> return 0;
> }
--
Regards,
Laurent Pinchart
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/6] clocksource: em_sti: convert to clk_prepare/unprepare
Hi Yoshii-san,
Thank you for the patch.
On Tuesday 24 September 2013 13:09:24 takas...@ops.dti.ne.jp wrote:
> From: Shinya Kuribayashi
>
> Add calls to clk_prepare and unprepare so that EMMA Mobile EV2 can
> migrate to the common clock framework.
>
> Signed-off-by: Shinya Kuribayashi
Acked-by: Laurent Pinchart
> ---
> drivers/clocksource/em_sti.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/clocksource/em_sti.c b/drivers/clocksource/em_sti.c
> index 3a5909c..9d17083 100644
> --- a/drivers/clocksource/em_sti.c
> +++ b/drivers/clocksource/em_sti.c
> @@ -78,7 +78,7 @@ static int em_sti_enable(struct em_sti_priv *p)
> int ret;
>
> /* enable clock */
> - ret = clk_enable(p->clk);
> + ret = clk_prepare_enable(p->clk);
> if (ret) {
> dev_err(&p->pdev->dev, "cannot enable clock\n");
> return ret;
> @@ -107,7 +107,7 @@ static void em_sti_disable(struct em_sti_priv *p)
> em_sti_write(p, STI_INTENCLR, 3);
>
> /* stop clock */
> - clk_disable(p->clk);
> + clk_disable_unprepare(p->clk);
> }
>
> static cycle_t em_sti_count(struct em_sti_priv *p)
--
Regards,
Laurent Pinchart
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2] ARM: shmobile: ape6evm: fix incorrect placement of __initdata tag
On Monday 30 September 2013 17:34:36 Bartlomiej Zolnierkiewicz wrote:
> __initdata tag should be placed between the variable name and equal
> sign for the variable to be placed in the intended .init.data section.
>
> Signed-off-by: Bartlomiej Zolnierkiewicz
> Signed-off-by: Kyungmin Park
Acked-by: Laurent Pinchart
> ---
> v2:
> - use __initdata as it is OK to do it
>
> arch/arm/mach-shmobile/board-ape6evm.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/arm/mach-shmobile/board-ape6evm.c
> b/arch/arm/mach-shmobile/board-ape6evm.c index 7627385..8954f55 100644
> --- a/arch/arm/mach-shmobile/board-ape6evm.c
> +++ b/arch/arm/mach-shmobile/board-ape6evm.c
> @@ -86,7 +86,7 @@ static struct gpio_keys_button gpio_buttons[] = {
> GPIO_KEY(KEY_VOLUMEDOWN,329,"S21"),
> };
>
> -static struct __initdata gpio_keys_platform_data ape6evm_keys_pdata = {
> +static struct gpio_keys_platform_data ape6evm_keys_pdata __initdata = {
> .buttons= gpio_buttons,
> .nbuttons = ARRAY_SIZE(gpio_buttons),
> };
--
Regards,
Laurent Pinchart
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 3/6] sh: clkfwk: Select sh-/common- clkfwk alternatively
Hi Yoshii-san, Thank you for the patch. On Tuesday 24 September 2013 13:12:06 takas...@ops.dti.ne.jp wrote: > Make sh clock framework core depend on HAVE_MACH_CLKDEV, and > set it > - y on sh for backward compatibility > - !CONFIG_COMMON_CLK on sh-mobile > This is a preparation for migration to common clock framework > from sh clock framework on sh-mobile. While I agree with this patch, I believe the use of the HAVE_MACH_CLKDEV configuration option to select whether to compile core.o in is a bit of an abuse. I would have created a new configuration option (SH_CLK_CORE in drivers/sh/Kconfig for instance) as 'def_bool y' that depends on SH || (ARM && !COMMON_CLK). However, as all ARCH_SHMOBILE platforms should be converted to the common clock framework, this is only temporary and could be revisited later, so I'm fine with keeping the patch as-is. > Signed-off-by: Takashi Yoshii > --- > arch/arm/Kconfig| 2 +- > arch/sh/Kconfig | 1 + > drivers/sh/clk/Makefile | 3 +-- > 3 files changed, 3 insertions(+), 3 deletions(-) > > diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig > index 3f7714d..53044ca 100644 > --- a/arch/arm/Kconfig > +++ b/arch/arm/Kconfig > @@ -650,7 +650,7 @@ config ARCH_SHMOBILE > select HAVE_ARM_SCU if SMP > select HAVE_ARM_TWD if SMP > select HAVE_CLK > - select HAVE_MACH_CLKDEV > + select HAVE_MACH_CLKDEV if !COMMON_CLK > select HAVE_SMP > select MIGHT_HAVE_CACHE_L2X0 > select MULTI_IRQ_HANDLER > diff --git a/arch/sh/Kconfig b/arch/sh/Kconfig > index 224f4bc..f57e47f 100644 > --- a/arch/sh/Kconfig > +++ b/arch/sh/Kconfig > @@ -41,6 +41,7 @@ config SUPERH > select MODULES_USE_ELF_RELA > select OLD_SIGSUSPEND > select OLD_SIGACTION > + select HAVE_MACH_CLKDEV > help > The SuperH is a RISC processor targeted for use in embedded systems > and consumer electronics; it was also used in the Sega Dreamcast > diff --git a/drivers/sh/clk/Makefile b/drivers/sh/clk/Makefile > index 5d15ebf..e73b031 100644 > --- a/drivers/sh/clk/Makefile > +++ b/drivers/sh/clk/Makefile > @@ -1,3 +1,2 @@ > -obj-y:= core.o > - > +obj-$(CONFIG_HAVE_MACH_CLKDEV) += core.o > obj-$(CONFIG_SH_CLK_CPG) += cpg.o -- Regards, Laurent Pinchart -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH v6] gpio: pcf857x: Add OF support
Add DT bindings for the pcf857x-compatible chips and parse the device
tree node in the driver.
Signed-off-by: Laurent Pinchart
---
.../devicetree/bindings/gpio/gpio-pcf857x.txt | 71 ++
drivers/gpio/gpio-pcf857x.c| 44 +++---
2 files changed, 107 insertions(+), 8 deletions(-)
create mode 100644 Documentation/devicetree/bindings/gpio/gpio-pcf857x.txt
Changes since v5:
- Renamed pin to line in the DT bindings documentation
- Renamed pins-initial-states DT property to lines-initial-states
Changes since v4:
- Don't try to get ngpio from of_device_id data, we already get it from
i2c_device_id
Changes since v3:
- Get rid of the #ifdef CONFIG_OF in the probe function
- Give DT node priority over platform data
Changes since v2:
- Replace mention about interrupts software configuration in DT bindings
documentation with an explanation of the hardware configuration.
diff --git a/Documentation/devicetree/bindings/gpio/gpio-pcf857x.txt
b/Documentation/devicetree/bindings/gpio/gpio-pcf857x.txt
new file mode 100644
index 000..d63194a
--- /dev/null
+++ b/Documentation/devicetree/bindings/gpio/gpio-pcf857x.txt
@@ -0,0 +1,71 @@
+* PCF857x-compatible I/O expanders
+
+The PCF857x-compatible chips have "quasi-bidirectional" I/O lines that can be
+driven high by a pull-up current source or driven low to ground. This combines
+the direction and output level into a single bit per line, which can't be read
+back. We can't actually know at initialization time whether a line is
configured
+(a) as output and driving the signal low/high, or (b) as input and reporting a
+low/high value, without knowing the last value written since the chip came out
+of reset (if any). The only reliable solution for setting up line direction is
+thus to do it explicitly.
+
+Required Properties:
+
+ - compatible: should be one of the following.
+- "maxim,max7328": For the Maxim MAX7378
+- "maxim,max7329": For the Maxim MAX7329
+- "nxp,pca8574": For the NXP PCA8574
+- "nxp,pca8575": For the NXP PCA8575
+- "nxp,pca9670": For the NXP PCA9670
+- "nxp,pca9671": For the NXP PCA9671
+- "nxp,pca9672": For the NXP PCA9672
+- "nxp,pca9673": For the NXP PCA9673
+- "nxp,pca9674": For the NXP PCA9674
+- "nxp,pca9675": For the NXP PCA9675
+- "nxp,pcf8574": For the NXP PCF8574
+- "nxp,pcf8574a": For the NXP PCF8574A
+- "nxp,pcf8575": For the NXP PCF8575
+- "ti,tca9554": For the TI TCA9554
+
+ - reg: I2C slave address.
+
+ - gpio-controller: Marks the device node as a gpio controller.
+ - #gpio-cells: Should be 2. The first cell is the GPIO number and the second
+cell specifies GPIO flags, as defined in . Only
the
+GPIO_ACTIVE_HIGH and GPIO_ACTIVE_LOW flags are supported.
+
+Optional Properties:
+
+ - lines-initial-states: Bitmask that specifies the initial state of each
+ line. When a bit is set to zero, the corresponding line will be initialized
to
+ the input (pulled-up) state. When the bit is set to one, the line will be
+ initialized the the low-level output state. If the property is not specified
+ all lines will be initialized to the input state.
+
+ The I/O expander can detect input state changes, and thus optionally act as
+ an interrupt controller. When the expander interrupt line is connected all
the
+ following properties must be set. For more information please see the
+ interrupt controller device tree bindings documentation available at
+ Documentation/devicetree/bindings/interrupt-controller/interrupts.txt.
+
+ - interrupt-controller: Identifies the node as an interrupt controller.
+ - #interrupt-cells: Number of cells to encode an interrupt source, shall be
2.
+ - interrupt-parent: phandle of the parent interrupt controller.
+ - interrupts: Interrupt specifier for the controllers interrupt.
+
+
+Please refer to gpio.txt in this directory for details of the common GPIO
+bindings used by client devices.
+
+Example: PCF8575 I/O expander node
+
+ pcf8575: gpio@20 {
+ compatible = "nxp,pcf8575";
+ reg = <0x20>;
+ interrupt-parent = <&irqpin2>;
+ interrupts = <3 0>;
+ gpio-controller;
+ #gpio-cells = <2>;
+ interrupt-controller;
+ #interrupt-cells = <2>;
+ };
diff --git a/drivers/gpio/gpio-pcf857x.c b/drivers/gpio/gpio-pcf857x.c
index 54725a6..1535686 100644
--- a/drivers/gpio/gpio-pcf857x.c
+++ b/drivers/gpio/gpio-pcf857x.c
@@ -26,6 +26,8 @@
#include
#include
#include
+#include
+#include
#include
#include
@@ -49,6 +51,27 @@ static const struct i2c_device_id pcf857x_id[] = {
};
MODULE_DEVICE_TABLE(i2c, pcf857x_id);
+#ifdef CONFIG_OF
+static
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 > > > > > > > > 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-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v6 0/5] clk: clock deregistration support
*hw)
> +{
> + return ___clk_register(dev, hw, THIS_MODULE);
> +}
> +
> +struct clk *__devm_clk_register(struct device *dev, struct clk_hw *hw,
> + struct module *owner);
> +static inline struct clk *devm_clk_register(struct device *dev,
> + struct clk_hw *hw)
> +{
> + return __devm_clk_register(dev, hw, THIS_MODULE);
> +}
>
> void clk_unregister(struct clk *clk);
> void devm_clk_unregister(struct device *dev, struct clk *clk);
> -8<--
>
> Similarly it would need to be done for the remaining clk_register*()
> functions, which have much longer arguments list.
>
> It's a bit messy, since there is already __clk_register() function with
> double underscore prefix. Perhaps that could be renamed to something else so
> all the functions taking struct module * parameter are prefixed with double
> underscore.
>
> I would squash patches 3/5 and 4/5 in the next iteration, if it is decided
> to keep using dev->driver->owner.
--
Regards,
Laurent Pinchart
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/4] ARM: shmobile: r8a7790: add I2C support in Device Tree mode
Hi Guennadi,
Thank you for the patch.
On Monday 09 September 2013 18:03:54 Guennadi Liakhovetski wrote:
> This patch adds clocks and clock lookup entries for the four I2C
> controllers on r8a7790 and respective Device Tree nodes.
>
> Signed-off-by: Guennadi Liakhovetski
> ---
> arch/arm/boot/dts/r8a7790.dtsi | 36 +
> arch/arm/mach-shmobile/clock-r8a7790.c | 10
> 2 files changed, 46 insertions(+), 0 deletions(-)
>
> diff --git a/arch/arm/boot/dts/r8a7790.dtsi b/arch/arm/boot/dts/r8a7790.dtsi
> index 885f9f4..a5021112 100644
> --- a/arch/arm/boot/dts/r8a7790.dtsi
> +++ b/arch/arm/boot/dts/r8a7790.dtsi
> @@ -127,6 +127,42 @@
> interrupts = <0 0 4>, <0 1 4>, <0 2 4>, <0 3 4>;
> };
>
> + i2c0: i2c@e6508000 {
> + #address-cells = <1>;
> + #size-cells = <0>;
> + compatible = "renesas,i2c-rcar-h2";
> + reg = <0 0xe6508000 0 0x40>;
> + interrupt-parent = <&gic>;
> + interrupts = <0 287 0x4>;
Shouldn't you add state = "disabled" to all I2C controllers in order not to
enable the unused controllers by default ?
> + };
> +
> + i2c1: i2c@e6518000 {
> + #address-cells = <1>;
> + #size-cells = <0>;
> + compatible = "renesas,i2c-rcar-h2";
> + reg = <0 0xe6518000 0 0x40>;
> + interrupt-parent = <&gic>;
> + interrupts = <0 288 0x4>;
> + };
> +
> + i2c2: i2c@e653 {
> + #address-cells = <1>;
> + #size-cells = <0>;
> + compatible = "renesas,i2c-rcar-h2";
> + reg = <0 0xe653 0 0x40>;
> + interrupt-parent = <&gic>;
> + interrupts = <0 286 0x4>;
> + };
> +
> + i2c3: i2c@e654 {
> + #address-cells = <1>;
> + #size-cells = <0>;
> + compatible = "renesas,i2c-rcar-h2";
> + reg = <0 0xe654 0 0x40>;
> + interrupt-parent = <&gic>;
> + interrupts = <0 290 0x4>;
> + };
> +
> mmcif0: mmcif@ee20 {
> compatible = "renesas,sh-mmcif";
> reg = <0 0xee20 0 0x80>;
> diff --git a/arch/arm/mach-shmobile/clock-r8a7790.c
> b/arch/arm/mach-shmobile/clock-r8a7790.c index fc36d3d..8e5e90b 100644
> --- a/arch/arm/mach-shmobile/clock-r8a7790.c
> +++ b/arch/arm/mach-shmobile/clock-r8a7790.c
> @@ -52,6 +52,7 @@
> #define SMSTPCR5 0xe6150144
> #define SMSTPCR7 0xe615014c
> #define SMSTPCR8 0xe6150990
> +#define SMSTPCR9 0xe6150994
>
> #define SDCKCR 0xE6150074
> #define SD2CKCR 0xE6150078
> @@ -181,6 +182,7 @@ static struct clk div6_clks[DIV6_NR] = {
>
> /* MSTP */
> enum {
> + MSTP931, MSTP930, MSTP929, MSTP928,
> MSTP813,
> MSTP721, MSTP720,
> MSTP717, MSTP716,
> @@ -192,6 +194,10 @@ enum {
> };
>
> static struct clk mstp_clks[MSTP_NR] = {
> + [MSTP931] = SH_CLK_MSTP32(&hp_clk, SMSTPCR9, 31, 0), /* I2C0 */
> + [MSTP930] = SH_CLK_MSTP32(&hp_clk, SMSTPCR9, 30, 0), /* I2C1 */
> + [MSTP929] = SH_CLK_MSTP32(&hp_clk, SMSTPCR9, 29, 0), /* I2C2 */
> + [MSTP928] = SH_CLK_MSTP32(&hp_clk, SMSTPCR9, 28, 0), /* I2C3 */
> [MSTP813] = SH_CLK_MSTP32(&p_clk, SMSTPCR8, 13, 0), /* Ether */
> [MSTP721] = SH_CLK_MSTP32(&p_clk, SMSTPCR7, 21, 0), /* SCIF0 */
> [MSTP720] = SH_CLK_MSTP32(&p_clk, SMSTPCR7, 20, 0), /* SCIF1 */
> @@ -261,6 +267,10 @@ static struct clk_lookup lookups[] = {
> CLKDEV_DEV_ID("sh-sci.7", &mstp_clks[MSTP720]),
> CLKDEV_DEV_ID("sh-sci.8", &mstp_clks[MSTP717]),
> CLKDEV_DEV_ID("sh-sci.9", &mstp_clks[MSTP716]),
> + CLKDEV_DEV_ID("e6508000.i2c", &mstp_clks[MSTP931]),
> + CLKDEV_DEV_ID("e6518000.i2c", &mstp_clks[MSTP930]),
> + CLKDEV_DEV_ID("e653.i2c", &mstp_clks[MSTP929]),
> + CLKDEV_DEV_ID("e654.i2c", &mstp_clks[MSTP928]),
> CLKDEV_DEV_ID("r8a7790-ether", &mstp_clks[MSTP813]),
> CLKDEV_DEV_ID("rcar_thermal", &mstp_clks[MSTP522]),
> CLKDEV_DEV_ID("ee20.mmcif", &mstp_clks[MSTP315]),
--
Regards,
Laurent Pinchart
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/4] pinctrl: sh-pfc: r8a7790: add pin definitions for the I2C3 interface
Hi Guennadi,
Thank you for the patch.
On Monday 09 September 2013 18:03:53 Guennadi Liakhovetski wrote:
> There are four I2C interfaces on r8a7790, each of them can be connected to
> one of the two respective I2C controllers, e.g. interface #0 can be
> configured to work with I2C0 or with IIC0. Additionally some of those
> interfaces can also use one of several pin sets. Interface #3 is special,
> because it can be used in automatic mode for DVFS. It only has one set
> of pins available and those pins cannot be used for anything else, they
> also lack the GPIO function.
>
> This patch uses the sh-pfc ability to configure pins, not associated with
> GPIOs and adds support for I2C3 to the r8a7790 PFC set up.
Ulrich Hecht sent a patch titled "sh-pfc: r8a7790: Add I2C pin groups and
functions" that added pin groups for I2C1 and I2C2. The patch is available
from
git://linuxtv.org/pinchartl/fbdev.git pinmux/next
If you need to resubmit this patch due to my comments below, could you please
rebase it on top of that branch ?
> Signed-off-by: Guennadi Liakhovetski
> ---
> drivers/pinctrl/sh-pfc/pfc-r8a7790.c | 28
> 1 files changed, 28 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/pinctrl/sh-pfc/pfc-r8a7790.c
> b/drivers/pinctrl/sh-pfc/pfc-r8a7790.c index 64fcc006..c3c4d9b 100644
> --- a/drivers/pinctrl/sh-pfc/pfc-r8a7790.c
> +++ b/drivers/pinctrl/sh-pfc/pfc-r8a7790.c
> @@ -781,6 +781,8 @@ enum {
> ADICS_SAMP_MARK, DU2_CDE_MARK, QPOLB_MARK, SCIFA2_RXD_B_MARK,
> USB1_PWEN_MARK, AUDIO_CLKOUT_D_MARK, USB1_OVC_MARK,
> TCLK1_B_MARK,
> +
> + I2C3_SCL_MARK, I2C3_SDA_MARK,
> PINMUX_MARK_END,
> };
>
> @@ -1719,10 +1721,22 @@ static const u16 pinmux_data[] = {
> PINMUX_IPSR_DATA(IP16_6, AUDIO_CLKOUT_D),
> PINMUX_IPSR_DATA(IP16_7, USB1_OVC),
> PINMUX_IPSR_MODSEL_DATA(IP16_7, TCLK1_B, SEL_TMU1_1),
> +
> + PINMUX_DATA(I2C3_SCL_MARK, FN_SEL_IICDVFS_1),
> + PINMUX_DATA(I2C3_SDA_MARK, FN_SEL_IICDVFS_1),
You introduce a way to mux the I2C3 function on those two pins, but no way to
select the IICDVFS back. I don't think it's an issue, we can always add that
later when (if) needed. Linus, is that fine with you ?
> };
>
> +/* R8A7790 has 6 banks with 32 GPIOs in each = 192 GPIOs */
> +#define ROW_GROUP_A(r) ('Z' - 'A' + 1 + (r))
> +#define PIN_NUMBER(r, c) (((r) - 'A') * 16 + (c) + 200)
The BGA package has 31 columns, shouldn't you multiply the row number by 31
instead of 16 ?
As we have 192 GPIOs, shouldn't you use an offset of 192 instead of 200 ? This
doesn't matter too much I guess.
> +#define PIN_A_NUMBER(r, c) PIN_NUMBER(ROW_GROUP_A(r), c)
> +
> static struct sh_pfc_pin pinmux_pins[] = {
> PINMUX_GPIO_GP_ALL(),
> +
> + /* Pins not associated with a GPIO port */
> + SH_PFC_PIN_NAMED(ROW_GROUP_A('J'), 15, AJ15),
> + SH_PFC_PIN_NAMED(ROW_GROUP_A('H'), 15, AH15),
> };
>
> /* - DU RGB
> - */ @@
> -1990,6 +2004,14 @@ static const unsigned int hscif1_ctrl_b_pins[] = {
> static const unsigned int hscif1_ctrl_b_mux[] = {
> HRTS1_N_B_MARK, HCTS1_N_B_MARK,
> };
> +/* - I2C3
> --- */
> +static const unsigned int i2c3_pins[] = {
> + /* SCL, SDA */
> + PIN_A_NUMBER('J', 15), PIN_A_NUMBER('H', 15),
> +};
> +static const unsigned int i2c3_mux[] = {
> + I2C3_SCL_MARK, I2C3_SDA_MARK,
> +};
> /* - INTC
> --- */
> static const unsigned int intc_irq0_pins[] = {
> /* IRQ */
> @@ -3047,6 +3069,7 @@ static const struct sh_pfc_pin_group pinmux_groups[] =
> { SH_PFC_PIN_GROUP(hscif1_data_b),
> SH_PFC_PIN_GROUP(hscif1_clk_b),
> SH_PFC_PIN_GROUP(hscif1_ctrl_b),
> + SH_PFC_PIN_GROUP(i2c3),
> SH_PFC_PIN_GROUP(intc_irq0),
> SH_PFC_PIN_GROUP(intc_irq1),
> SH_PFC_PIN_GROUP(intc_irq2),
> @@ -3243,6 +3266,10 @@ static const char * const hscif1_groups[] = {
> "hscif1_ctrl_b",
> };
>
> +static const char * const i2c3_groups[] = {
> + "i2c3",
> +};
> +
> static const char * const intc_groups[] = {
> "intc_irq0",
> "intc_irq1",
> @@ -3469,6 +3496,7 @@ static const struct sh_pfc_function pinmux_functions[]
> = { SH_PFC_FUNCTION(eth),
> SH_PFC_FUNCTION(hscif0),
> SH_PFC_FUNCTION(hscif1),
> + SH_PFC_FUNCTION(i2c3),
> SH_PFC_FUNCTION(intc),
> SH_PFC_FUNCTION(mmc0),
> SH_PFC_FUNCTION(mmc1),
--
Regards,
Laurent Pinchart
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH] drivers: cma: Fix wrong CMA selected region size default value
Kconfig lists CMA_SIZE_SEL_ABSOLUTE as the default value fo the CMA selected region size, but that option isn't available in the defined choices. Set the default to CMA_SIZE_SEL_MBYTES instead. Signed-off-by: Laurent Pinchart --- drivers/base/Kconfig |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/drivers/base/Kconfig b/drivers/base/Kconfig index 08b4c52..b34b5cd 100644 --- a/drivers/base/Kconfig +++ b/drivers/base/Kconfig @@ -236,7 +236,7 @@ config CMA_SIZE_PERCENTAGE choice prompt "Selected region size" - default CMA_SIZE_SEL_ABSOLUTE + default CMA_SIZE_SEL_MBYTES config CMA_SIZE_SEL_MBYTES bool "Use mega bytes value only" -- Regards, Laurent Pinchart -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 2/2] drivers: dma-coherent: Fix typo in dma_mmap_from_coherent documentation
The function documentation incorrectly references dma_release_coherent. Fix it. Don't mention a specific function name as dma_mmap_from_coherent as multiple callers. Signed-off-by: Laurent Pinchart --- drivers/base/dma-coherent.c |5 ++--- 1 files changed, 2 insertions(+), 3 deletions(-) diff --git a/drivers/base/dma-coherent.c b/drivers/base/dma-coherent.c index 560a717..bc256b6 100644 --- a/drivers/base/dma-coherent.c +++ b/drivers/base/dma-coherent.c @@ -191,9 +191,8 @@ EXPORT_SYMBOL(dma_release_from_coherent); * This checks whether the memory was allocated from the per-device * coherent memory pool and if so, maps that memory to the provided vma. * - * Returns 1 if we correctly mapped the memory, or 0 if - * dma_release_coherent() should proceed with mapping memory from - * generic pools. + * Returns 1 if we correctly mapped the memory, or 0 if the caller should + * proceed with mapping memory from generic pools. */ int dma_mmap_from_coherent(struct device *dev, struct vm_area_struct *vma, void *vaddr, size_t size, int *ret) -- 1.7.8.6 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 1/2] drivers: dma-contiguous: Don't redefine SZ_1M
Use the definition from linux/sizes.h instead.
Signed-off-by: Laurent Pinchart
---
drivers/base/dma-contiguous.c |5 +
1 files changed, 1 insertions(+), 4 deletions(-)
diff --git a/drivers/base/dma-contiguous.c b/drivers/base/dma-contiguous.c
index 9a14694..612afcc 100644
--- a/drivers/base/dma-contiguous.c
+++ b/drivers/base/dma-contiguous.c
@@ -27,15 +27,12 @@
#include
#include
#include
+#include
#include
#include
#include
#include
-#ifndef SZ_1M
-#define SZ_1M (1 << 20)
-#endif
-
struct cma {
unsigned long base_pfn;
unsigned long count;
--
1.7.8.6
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[RFC] linux/time.h vs. sys/time.h mess (was [PATCH 1/2] [media] remove include/linux/dvb/dmx.h)
Hi Mauro, (CC'ing LKML) On Friday 19 October 2012 08:21:16 Mauro Carvalho Chehab wrote: > Em Fri, 19 Oct 2012 07:43:11 -0300 > > Mauro Carvalho Chehab escreveu: > > -#include > > -#include > > - > > -#endif /*_DVBDMX_H_*/ > > Just to not discard a valid comment on IRC, Laurent proposed that we > should investigate if we can, instead, move: > > #include > > to both dmx.h and videodev2.h, letting it to be included by both userspace > and Kernelspace. > > I remember this used to cause compilation breakage in the past, as some > userspace programs need to include and this used to conflict > with . > > I'm not sure if this got fixed there. if so, Laurent has a point. It's still not solved, but that's what the proper fix should be. Several UAPI headers use struct timeval or struct timespec. Kernel code and user space code thus need to include the header(s) that define those structures, either directly or indirectly. In kernel space struct timeval and struct timespec are defined in include/uapi/linux/time.h. In user space they're defined in . No proper conditional compilation exists in those headers to guard against multiple definitions, so they can't be included together. On the kernel side isn't available, so we can include in the headers that use the timeval and timespec structures. This "self-contained" headers mechanism avoids forcing all users of those headers to explicitly include . However, this then breaks user space applications that include both and a kernel header that includes . The way we've dealt with that until now is by including either or depending on __KERNEL__ #ifdef __KERNEL__ #include #else #include #endif in our user-facing headers. The recent UAPI disintegration patches resulted in nearly empty headers in include/linux/ that just #include both and the corresponding UAPI header. For instance include/linux/videodev2.h is now just #ifndef __LINUX_VIDEODEV2_H #define __LINUX_VIDEODEV2_H #include /* need struct timeval */ #include #endif /* __LINUX_VIDEODEV2_H */ Patches have been posted to remove those headers and push the #include one level up, which breaks the "self-contained" headers concept. How could we fix this ? Are there legitimate users of in user space ? A quick grep in glibc doesn't reveal anything. -- Regards, Laurent Pinchart -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/1] usb: gadget: Don't attempt to dequeue requests for a disabled USB endpoint on Freescale hardware
Hi, On Monday 22 October 2012 03:33:19 Li Yang-R58472 wrote: > On Saturday, October 20, 2012 1:37 AM Felipe Balbi wrote: > > On Fri, Oct 19, 2012 at 06:19:26PM +0100, Simon Haggett wrote: > > > Some gadget drivers may attempt to dequeue requests for an endpoint > > > that has already been disabled. For example, in the UVC gadget driver, > > > uvc_function_set_alt() will call usb_ep_disable() when alt setting 0 > > > is selected. When the userspace application subsequently issues the > > > VIDIOC_STREAMOFF ioctl, uvc_video_enable() invokes usb_ep_dequeue() to > > > > ensure that all requests have been cancelled. > > > > bug is on uvc gadget, then. Laurent ? We've discussed this topic a couple of months before. I believe that's not a bug. http://68.183.106.108/lists/linux-usb/msg68869.html > > Also, fsl should be removed from the tree, I'm trying to persuade iMX > > folks to use drivers/usb/chipidea instead. > > Besides the iMX usage, the driver is also being used by many Freescale > PowerPC/Coldfire SoCs. I agree that it's ideal to move to a common driver. > But it is a large task to make the chipidea driver works for all the > hardware that fsl_udc had supported and been tested on. > > > > For the Freescale High Speed Dual-Role USB controller, > > > fsl_ep_dequeue() provides the implementation of usb_ep_dequeue(). If > > > this is called for a disabled endpoint, a kernel oops will occur when > > > > the ep->ep.desc field is dereferenced (by ep_index()). > > > > > fsl_ep_disable() sets this field to NULL, as well as deleting all > > > pending requests for the endpoint. > > > > > > This patch adds an additional check to fsl_ep_dequeue() to ensure that > > > the endpoint has not already been disabled before attempting to dequeue > > > > requests. > > > > > Signed-off-by: Simon Haggett > > > --- > > > > > > drivers/usb/gadget/fsl_udc_core.c |5 - > > > 1 files changed, 4 insertions(+), 1 deletions(-) > > > > > > diff --git a/drivers/usb/gadget/fsl_udc_core.c > > > b/drivers/usb/gadget/fsl_udc_core.c > > > index 6ae70cb..acd513b 100644 > > > --- a/drivers/usb/gadget/fsl_udc_core.c > > > +++ b/drivers/usb/gadget/fsl_udc_core.c > > > @@ -955,7 +955,10 @@ static int fsl_ep_dequeue(struct usb_ep *_ep, > > struct usb_request *_req) > > > > > int ep_num, stopped, ret = 0; > > > u32 epctrl; > > > > > > - if (!_ep || !_req) > > > + /* Ensure that the ep and request are valid, and the ep is not > > > + * disabled > > > + */ > > > + if (!_ep || !_req || !ep->ep.desc) > > > return -EINVAL; Shouldn't that last check be done with a lock taken ? > > > spin_lock_irqsave(&ep->udc->lock, flags); -- Regards, Laurent Pinchart -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH RESEND] media: davinci: vpbe: fix build warning
Hi Prabhakar,
On Monday 22 October 2012 17:47:51 Prabhakar Lad wrote:
> From: Lad, Prabhakar
>
> Warnings were generated because of the following commit changed data type
> for address pointer
>
> 195bbca ARM: 7500/1: io: avoid writeback addressing modes for __raw_
> accessors add __iomem annotation to fix following warnings
>
> drivers/media/platform/davinci/vpbe_osd.c: In function ‘osd_read’:
> drivers/media/platform/davinci/vpbe_osd.c:49:2: warning: passing
> argument 1 of ‘__raw_readl’ makes pointer from integer without a cast
> [enabled by default] arch/arm/include/asm/io.h:104:19: note: expected
> ‘const volatile
> void *’ but argument is of type ‘long unsigned int’
>
> Signed-off-by: Lad, Prabhakar
> Signed-off-by: Manjunath Hadli
> ---
> Resending the patch since, it didn't reach the DLOS mailing list.
>
> drivers/media/platform/davinci/vpbe_osd.c | 16
> 1 files changed, 8 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/media/platform/davinci/vpbe_osd.c
> b/drivers/media/platform/davinci/vpbe_osd.c index bba299d..9ab9280 100644
> --- a/drivers/media/platform/davinci/vpbe_osd.c
> +++ b/drivers/media/platform/davinci/vpbe_osd.c
> @@ -46,14 +46,14 @@ static inline u32 osd_read(struct osd_state *sd, u32
> offset) {
> struct osd_state *osd = sd;
>
> - return readl(osd->osd_base + offset);
> + return readl(IOMEM(osd->osd_base + offset));
A better fix, in my opinion, would be to change the osd->osd_base field to be
a void __iomem * instead of long unsigned int.
> }
>
> static inline u32 osd_write(struct osd_state *sd, u32 val, u32 offset)
> {
> struct osd_state *osd = sd;
>
> - writel(val, osd->osd_base + offset);
> + writel(val, IOMEM(osd->osd_base + offset));
>
> return val;
> }
> @@ -63,9 +63,9 @@ static inline u32 osd_set(struct osd_state *sd, u32 mask,
> u32 offset) struct osd_state *osd = sd;
>
> u32 addr = osd->osd_base + offset;
> - u32 val = readl(addr) | mask;
> + u32 val = readl(IOMEM(addr)) | mask;
>
> - writel(val, addr);
> + writel(val, IOMEM(addr));
>
> return val;
> }
> @@ -75,9 +75,9 @@ static inline u32 osd_clear(struct osd_state *sd, u32
> mask, u32 offset) struct osd_state *osd = sd;
>
> u32 addr = osd->osd_base + offset;
> - u32 val = readl(addr) & ~mask;
> + u32 val = readl(IOMEM(addr)) & ~mask;
>
> - writel(val, addr);
> + writel(val, IOMEM(addr));
>
> return val;
> }
> @@ -88,9 +88,9 @@ static inline u32 osd_modify(struct osd_state *sd, u32
> mask, u32 val, struct osd_state *osd = sd;
>
> u32 addr = osd->osd_base + offset;
> - u32 new_val = (readl(addr) & ~mask) | (val & mask);
> + u32 new_val = (readl(IOMEM(addr)) & ~mask) | (val & mask);
>
> - writel(new_val, addr);
> + writel(new_val, IOMEM(addr));
>
> return new_val;
> }
--
Regards,
Laurent Pinchart
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v4 1/2] iommu/shmobile: Add iommu driver for Renesas IPMMU modules
eak;
> + default:
> + ret = -EINVAL;
> + }
> + if (!ret && atomic_read(&priv->active)) {
> + wmb();
> + ipmmu_tlb_flush(ipmmu_access_device);
> + l2realfree(&l2);
> + }
> + return ret;
> +}
> +
> +static size_t shmobile_iommu_unmap(struct iommu_domain *domain,
> +unsigned long iova, size_t size)
> +{
> + struct shmobile_iommu_priv_pgtable l2 = { .pgtable = NULL };
> + struct shmobile_iommu_priv *priv = domain->priv;
> + unsigned int l1index, l2index, i;
> + uint32_t l2entry = 0;
> + size_t ret = 0;
> +
> + l1index = iova >> 20;
> + if (!(iova & 0xF) && size >= 0x10) {
> + spin_lock(&priv->map_lock);
> + l2free(priv, l1index, &l2);
> + spin_unlock(&priv->map_lock);
> + ret = 0x10;
> + goto done;
> + }
> + l2index = (iova >> 12) & 0xff;
> + spin_lock(&priv->map_lock);
> + if (priv->l2[l1index].pgtable)
> + l2entry = priv->l2[l1index].pgtable[l2index];
> + switch (l2entry & 3) {
> + case 1:
> + if (l2index & 0xf)
> + break;
> + for (i = 0; i < 0x10; i++)
> + priv->l2[l1index].pgtable[l2index + i] = 0;
> + ret = 0x1;
> + break;
> + case 2:
> + priv->l2[l1index].pgtable[l2index] = 0;
> + ret = 0x1000;
> + break;
> + }
> + spin_unlock(&priv->map_lock);
> +done:
> + if (ret && atomic_read(&priv->active)) {
> + wmb();
> + ipmmu_tlb_flush(ipmmu_access_device);
> + l2realfree(&l2);
> + }
> + return ret;
> +}
> +
> +static phys_addr_t shmobile_iommu_iova_to_phys(struct iommu_domain *domain,
> +unsigned long iova)
> +{
> + struct shmobile_iommu_priv *priv = domain->priv;
> + uint32_t l1entry = 0, l2entry = 0;
> + unsigned int l1index, l2index;
> +
> + l1index = iova >> 20;
> + l2index = (iova >> 12) & 0xff;
> + spin_lock(&priv->map_lock);
> + if (priv->l2[l1index].pgtable)
> + l2entry = priv->l2[l1index].pgtable[l2index];
> + else
> + l1entry = priv->l1.pgtable[l1index];
> + spin_unlock(&priv->map_lock);
> + switch (l2entry & 3) {
> + case 1:
> + return (l2entry & ~0x) | (iova & 0x);
> + case 2:
> + return (l2entry & ~0xfff) | (iova & 0xfff);
> + default:
> + if ((l1entry & 3) == 2)
> + return (l1entry & ~0xf) | (iova & 0xf);
> + return 0;
> + }
> +}
> +
> +static struct iommu_ops shmobile_iommu_ops = {
> + .domain_init = shmobile_iommu_domain_init,
> + .domain_destroy = shmobile_iommu_domain_destroy,
> + .attach_dev = shmobile_iommu_attach_device,
> + .detach_dev = shmobile_iommu_detach_device,
> + .map = shmobile_iommu_map,
> + .unmap = shmobile_iommu_unmap,
> + .iova_to_phys = shmobile_iommu_iova_to_phys,
> + .pgsize_bitmap = 0x111000,
> +};
> +
> +static int shmobile_iommu_attach_all_devices(void)
> +{
> + struct device *dev;
> + int ret = 0;
> +
> + spin_lock(&lock_add);
> + iommu_mapping = arm_iommu_create_mapping(&platform_bus_type, 0x0,
> + L1_LEN << 20, 0);
> + if (IS_ERR_OR_NULL(iommu_mapping)) {
> + ret = PTR_ERR(iommu_mapping);
> + goto err;
> + }
> + for (dev = ipmmu_devices; dev; dev = dev->archdata.iommu) {
> + if (arm_iommu_attach_device(dev, iommu_mapping))
> + pr_err("arm_iommu_attach_device failed\n");
> + }
> +err:
> + spin_unlock(&lock_add);
> + return 0;
> +}
> +
> +void ipmmu_add_device(struct device *dev)
> +{
> + spin_lock(&lock_add);
> + dev->archdata.iommu = ipmmu_devices;
> + ipmmu_devices = dev;
That looks a bit hackish to me. I'd like to suggest a different approach, that
would be compatible with supporting multiple IPMMU instances.
dev->archdata.iommu should point to a new sh_ipmmu_arch_data structure that
would contain an IPMMU name (const char *) and a pointer to a struct
shmobile_iommu_priv.
ipmmu_add_device() would take a new IPMMU name argument, allocate an
sh_ipmmu_arch_data instance dynamically and initialize its name field to the
name passed to the function. The shmobile_iommu_priv pointer would be set to
NULL. No other operation would be performed (you will likely get rid of the
global ipmmu_devices and iommu_mapping variables).
Then, the attach_dev operation handler would retrieve the dev->archdata.iommu
pointer, cast that to an sh_ipmmu_arch_data, and retrieve the IPMMU associated
with the name (either by walking a driver-global list of IPMMUs, or by using
driver_find_device()).
This mechanism would get rid of several global variables in the driver
(several of them would move to the shmobile_ipmmu_priv structure - which I
would have named shmobile_ipmmu or even sh_ipmmu, but that's up to you) and
add support for several IPMMU instances (there's 3 of them in the sh7372, even
if we only need to support one right now it's still a good practice to design
the driver in a way that multiple instances can be supported).
Could you try to rework the driiver in that direction ? You can have a look at
the OMAP IOMMU driver if you need sample code, and obviously feel free to
contact me if you have any question.
> + if (!IS_ERR_OR_NULL(iommu_mapping)) {
> + if (arm_iommu_attach_device(dev, iommu_mapping))
> + pr_err("arm_iommu_attach_device failed\n");
> + }
> + spin_unlock(&lock_add);
> +}
> +
> +int ipmmu_iommu_init(struct device *dev)
> +{
> + dma_set_coherent_mask(dev, DMA_BIT_MASK(32));
> + l1pool = dma_pool_create("shmobile-iommu-pgtable1", dev,
> + L1_SIZE, L1_ALIGN, 0);
> + if (!l1pool)
> + goto nomem_pool1;
> + l2pool = dma_pool_create("shmobile-iommu-pgtable2", dev,
> + L2_SIZE, L2_ALIGN, 0);
> + if (!l2pool)
> + goto nomem_pool2;
> + spin_lock_init(&lock);
> + attached = NULL;
> + ipmmu_access_device = dev;
> + bus_set_iommu(&platform_bus_type, &shmobile_iommu_ops);
> + if (shmobile_iommu_attach_all_devices())
> + pr_err("shmobile_iommu_attach_all_devices failed\n");
> + return 0;
> +nomem_pool2:
> + dma_pool_destroy(l1pool);
> +nomem_pool1:
> + return -ENOMEM;
> +}
--
Regards,
Laurent Pinchart
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v4] fbdev: sh_mobile_lcdc: use dma_mmap_coherent
Hi Eiraku-san, On Monday 10 December 2012 19:31:25 Hideki EIRAKU wrote: > On Thu, 16 Aug 2012 14:16:32 +0200 Laurent Pinchart wrote: > > On Thursday 16 August 2012 19:13:20 Hideki EIRAKU wrote: > >> fb_mmap() implemented in fbmem.c uses smem_start as the physical > >> address of the frame buffer. In the sh_mobile_lcdc driver, the > >> smem_start is a dma_addr_t that is not a physical address when IOMMU is > >> enabled. dma_mmap_coherent() maps the address correctly. > >> > >> Signed-off-by: Hideki EIRAKU > > > > Acked-by: Laurent Pinchart > > > > I will push the patch to v3.7 through my tree. > > I'd like to use this patch to test IOMMU implementation of Renesas > IPMMU. But I could not find it in v3.7-rc8. Could you please tell me if > this has already been merged somewhere or not? I'm afraid the patch got delayed to v3.8, sorry about that. I've sent a pull request for v3.8, the patch is currently in linux-next (bf10a53765b4435a5349a92a5a51753902ed86f1) and will be merged during the next merge window (which should open in the very near future). -- Regards, Laurent Pinchart -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v4 1/2] iommu/shmobile: Add iommu driver for Renesas IPMMU modules
Hi Eiraku-san,
On Tuesday 11 December 2012 19:10:42 Hideki EIRAKU wrote:
> On Mon, 10 Dec 2012 16:55:58 +0100 Laurent Pinchart wrote:
> > On Monday 15 October 2012 17:34:52 Hideki EIRAKU wrote:
> >> This is the Renesas IPMMU driver and IOMMU API implementation.
> >>
> >> The IPMMU module supports the MMU function and the PMB function.
> >
> > That sentence make me believe that both MMU and PMB were supported by the
> > driver, as "module" often refers to Linux kernel modules in this context.
> > Maybe you could replace "module" by "hardware module".
>
> OK,
>
> >> The MMU function provides address translation by pagetable compatible
> >> with ARMv6. The PMB function provides address translation including
> >> tile-linear translation. This patch implements the MMU function.
> >>
> >> The iommu driver does not register a platform driver directly because:
> >> - the register space of the MMU function and the PMB function
> >> have a common register (used for settings flush), so they should
> >> ideally have a way to appropriately share this register.
> >> - the MMU function uses the IOMMU API while the PMB function does not.
> >> - the two functions may be used independently.
> >>
> >> Signed-off-by: Hideki EIRAKU
> >> ---
> >>
> >> arch/arm/mach-shmobile/Kconfig |6 +
> >> arch/arm/mach-shmobile/Makefile |3 +
> >> arch/arm/mach-shmobile/include/mach/ipmmu.h | 16 ++
> >> arch/arm/mach-shmobile/ipmmu.c | 150
> >> drivers/iommu/Kconfig | 56 +
> >> drivers/iommu/Makefile |1 +
> >> drivers/iommu/shmobile-iommu.c | 352 +
> >> 7 files changed, 584 insertions(+), 0 deletions(-)
> >> create mode 100644 arch/arm/mach-shmobile/include/mach/ipmmu.h
> >> create mode 100644 arch/arm/mach-shmobile/ipmmu.c
> >> create mode 100644 drivers/iommu/shmobile-iommu.c
> >
> > What is the reason for splitting the driver in two files ? Can't you put
> > all the code in drivers/iommu/shmobile-iommu.c ? Storing driver code in
> > arch/* is discouraged.
>
> The reason is that I described in the above text. The PMB function is
> completely different from the MMU function but both functions are on
> the same IPMMU hardware module and sharing the register space. I think
> that a driver using the PMB part which is not yet released should not
> depend on the Linux's iommu interface, so I split the driver in two
> files: the IPMMU platform driver part (in arch/arm/mach-shmobile/) and
> Linux's iommu part (in drivers/iommu/). For the IPMMU platform driver part,
> do you have any suggestions other than arch/* where this should go? It is a
> generic platform device.
I think both parts should go to drivers/iommu/. You can keep the code split
across two files, but I think you should register a single platform driver.
The IPMMU is a single hardware module, so it should be handled by a single
driver. That driver can expose two different APIs (IOMMU and whatever API will
be used for PMB), and you can make those APIs selectable as Kconfig options,
but they should in my opinion be implemented in a single driver.
> >> + * You should have received a copy of the GNU General Public License
> >> + * along with this program; if not, write to the Free Software
> >> + * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301
> >> USA
> >
> > You can remove this last paragraph, we don't want to patch every file in
> > the kernel if the FSF moves to a new building :-)
>
> OK, I will remove the paragraph.
>
> >> + for (dev = ipmmu_devices; dev; dev = dev->archdata.iommu) {
> >> + if (arm_iommu_attach_device(dev, iommu_mapping))
> >> + pr_err("arm_iommu_attach_device failed\n");
> >> + }
> >> +err:
> >> + spin_unlock(&lock_add);
> >> + return 0;
> >> +}
> >> +
> >> +void ipmmu_add_device(struct device *dev)
> >> +{
> >> + spin_lock(&lock_add);
> >> + dev->archdata.iommu = ipmmu_devices;
> >> + ipmmu_devices = dev;
> >
> > That looks a bit hackish to me. I'd like to suggest a different approach,
> > that would be compatible with supporting multiple IPMMU instances.
> >
> > dev->archdata.iommu should point to a new sh_ipmmu_arch_data structure
> > that would contain an IPMMU name (const char
