Re: [PATCH] usb: Make USB persist default configurable
On Tue, Mar 19, 2013 at 7:56 AM, Alan Stern wrote: > > On Mon, 18 Mar 2013, Greg Kroah-Hartman wrote: > > > On Mon, Mar 18, 2013 at 05:02:19PM -0700, Julius Werner wrote: > > > > Why can't you just revert this in userspace? Isn't that easier than > > > > doing a kernel patch and providing an option that we need to now > > > > maintain for pretty much forever? > > > > > > I could solve it in userspace, but that really feels like a hacky > > > workaround and not a long term solution. It would mean that every new > > > device starts with persist enabled and stays that way for a few > > > milliseconds (maybe up to seconds if it's connected on boot), until > > > userspace gets around to disable it again... opening the possibility > > > for very weird race conditions and bugs with drivers/devices that > > > don't work with persist. > > > > What drivers/devices don't work with persist? We need to know that now, > > otherwise all other distros and users have problems, right? > > > > > This default is a policy that really resides in the kernel, it has > > > changed in the past, and since there is no definitive better choice > > > for all cases I thought making it configurable is the right thing to > > > do. > > > > Too many options can be a bad thing. > > > > I think Alan made this a "always on" option, so I'd like to get his > > opinion on it. Alan? > > Originally the "persist" attribute defaulted to "off". Linus disliked > this (at least, he disliked it for mass-storage devices) and so at his > request the default was changed to "on". There didn't seem to be any > reason to treat other devices differently from mass-storage devices; > consequently the default is now "on" for everything. > > Julius's commit message mentions the disadvantage of "persist": Resume > times can be increased. But it doesn't mention the chief advantage: > Filesystems stored on USB devices are much less likely to be lost > across suspends. > > The races mentioned above don't seem to be very dangerous. How likely > is it that the system will be suspended within a few milliseconds of > probing a new USB device? For laptops, if the suspend/resume is triggered by the lid open/close detection, this is somewhat likely and bit us in the past : the classical use case I have encountered is a back-to-back suspend triggered by the user opening the lid then closing it again in the next 2 or 3 seconds because he has changed is mind (damn user...), might be also triggered by lid hall sensor missing proper debouncing (but in that case, the mechanical time constant is often shorter than the latency of resuming USB devices). > > As for buggy devices and drivers that can't handle persist, we have > better ways of dealing with them. Buggy devices can get a quirk flag > (USB_QUIRK_RESET). Buggy drivers should be fixed. > > Alan Stern > -- Vincent -- 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] hwmon: (lm90) Add device tree support
Add support to instantiate LM90-compatible sensors from a device-tree configuration. When the kernel has device tree support, we avoid doing the auto-detection as probing the busses might mess-up sensitive I2C devices or trigger long timeouts on non-functional busses. Signed-off-by: Vincent Palatin --- .../devicetree/bindings/i2c/trivial-devices.txt| 19 + .../devicetree/bindings/vendor-prefixes.txt| 1 + drivers/hwmon/lm90.c | 47 +- 3 files changed, 65 insertions(+), 2 deletions(-) diff --git a/Documentation/devicetree/bindings/i2c/trivial-devices.txt b/Documentation/devicetree/bindings/i2c/trivial-devices.txt index 446859f..4d991ca 100644 --- a/Documentation/devicetree/bindings/i2c/trivial-devices.txt +++ b/Documentation/devicetree/bindings/i2c/trivial-devices.txt @@ -10,6 +10,7 @@ document for it just like any other devices. Compatible Vendor / Chip == = ad,ad7414 SMBus/I2C Digital Temperature Sensor in 6-Pin SOT with SMBus Alert and Over Temperature Pin +ad,adm1032 +/-1C Remote and local system temperature monitor ad,adm9240 ADM9240: Complete System Hardware Monitor for uProcessor-Based Systems adi,adt7461+/-1C TDM Extended Temp Range I.C adt7461+/-1C TDM Extended Temp Range I.C @@ -35,16 +36,33 @@ fsl,mc13892 MC13892: Power Management Integrated Circuit (PMIC) for i.MX35/51 fsl,mma8450MMA8450Q: Xtrinsic Low-power, 3-axis Xtrinsic Accelerometer fsl,mpr121 MPR121: Proximity Capacitive Touch Sensor Controller fsl,sgtl5000 SGTL5000: Ultra Low-Power Audio Codec +gmt,g781 +/-1C Remote and local temperature sensor maxim,ds1050 5 Bit Programmable, Pulse-Width Modulator maxim,max1237 Low-Power, 4-/12-Channel, 2-Wire Serial, 12-Bit ADCs maxim,max6625 9-Bit/12-Bit Temperature Sensors with I²C-Compatible Serial Interface +maxim,max6646 +145C Precision SMBus-Compatible Remote/Local Sensors +maxim,max6647 +145C Precision SMBus-Compatible Remote/Local Sensors +maxim,max6649 +145C Precision SMBus-Compatible Remote/Local Sensors +maxim,max6657 +/-1C SMBus-Compatible Remote/Local Sensors +maxim,max6658 +/-1C SMBus-Compatible Remote/Local Sensors +maxim,max6659 +/-1C SMBus-Compatible Remote/Local Sensors +maxim,max6680 +/-1C Fail-Safe Remote/Local Temperature Sensors +maxim,max6681 +/-1C Fail-Safe Remote/Local Temperature Sensors +maxim,max6695 Dual Remote/Local Temperature Sensors +maxim,max6696 Dual Remote/Local Temperature Sensors mc,rv3029c2Real Time Clock Module with I2C-Bus national,lm75 I2C TEMP SENSOR national,lm80 Serial Interface ACPI-Compatible Microprocessor System Hardware Monitor +national,lm86 +/-0.75C Accurate, Remote Diode and Local Digital Temperature Sensor with Two-Wire Interface +national,lm89 +/-0.75C Remote and Local Digital Temperature Sensor with Two-Wire Interface-Wire Interface +national,lm90 +/-3C Accurate, Remote Diode and Local Digital Temperature Sensor with Two-Wire Interface national,lm92 ±0.33°C Accurate, 12-Bit + Sign Temperature Sensor and Thermal Window Comparator with Two-Wire Interface +national,lm99 +/-1C Accurate, Remote Diode and Local Digital Temperature Sensor with Two-Wire Interface nxp,pca9556Octal SMBus and I2C registered interface nxp,pca95578-bit I2C-bus and SMBus I/O port with reset nxp,pcf8563Real-time clock/calendar +nxp,sa56004remote/local digital temperature sensor with overtemperature alarms +onnn,nct1008 +/-1C Temperature Monitor with Series Resistance Cancellation ovti,ov5642OV5642: Color CMOS QSXGA (5-megapixel) Image Sensor with OmniBSI and Embedded TrueFocus pericom,pt7c4338 Real-time Clock Module plx,pex864848-Lane, 12-Port PCI Express Gen 2 (5.0 GT/s) Switch @@ -59,3 +77,4 @@ taos,tsl2550 Ambient Light Sensor with SMBUS/Two Wire Serial Interface ti,tsc2003 I2C Touch-Screen Controller ti,tmp102 Low Power Digital Temperature Sensor with SMBUS/Two Wire Serial Interface ti,tmp275 Digital Temperature Sensor +winbond,w83l771H/W Monitor IC diff --git a/Documentation/devicetree/bindings/vendor-prefixes.txt b/Documentation/devicetree/bindings/vendor-prefixes.txt index 902b1b1..2074699 100644 --- a/Documentation/devicetree/bindings/vendor-prefixes.txt +++ b/Documentation/devicetree/bindings/vendor-prefixes.txt @@ -23,6 +23,7 @@ est ESTeem Wireless Modems fslFreescale Semiconductor GEFanucGE Fanuc Intelligent Platforms Embedded Systems, Inc. gefGE Fanuc Intelligent Platforms Embedded Systems, Inc. +gmtGlobal Mixed-mode
[PATCH] USB: ohci-exynos: initialize registers pointer earlier
In the former code, we have a race condition between the first interrupt and the regs field initilization in the usb_hcd structure. If the OHCI irq fires before hcd->regs is set, we are getting a null pointer dereference in ohci_irq. When calling usb_add_hcd(), it first executes the reset() callback, then enables the ohci interrupt, and finally executes the start() callback. So moving the ohci_init() call which actually initializes the reg field from start() to reset() should remove the race. Tested by enabling the external HSIC hub in the bootloader on an exynos5 machine and booting. With the former code, this triggers an early interrupt about 50% of the boots and a subsequent kernel panic in ohci_irq when trying to access the registers. Cc: Olof Johansson Cc: Doug Anderson Cc: Arjun.K.V Cc: Vikas Sajjan Cc: Abhilash Kesavan Signed-off-by: Vincent Palatin --- drivers/usb/host/ohci-exynos.c | 10 ++ 1 files changed, 6 insertions(+), 4 deletions(-) diff --git a/drivers/usb/host/ohci-exynos.c b/drivers/usb/host/ohci-exynos.c index 20a5008..f04cfde 100644 --- a/drivers/usb/host/ohci-exynos.c +++ b/drivers/usb/host/ohci-exynos.c @@ -23,6 +23,11 @@ struct exynos_ohci_hcd { struct clk *clk; }; +static int ohci_exynos_reset(struct usb_hcd *hcd) +{ + return ohci_init(hcd_to_ohci(hcd)); +} + static int ohci_exynos_start(struct usb_hcd *hcd) { struct ohci_hcd *ohci = hcd_to_ohci(hcd); @@ -30,10 +35,6 @@ static int ohci_exynos_start(struct usb_hcd *hcd) ohci_dbg(ohci, "ohci_exynos_start, ohci:%p", ohci); - ret = ohci_init(ohci); - if (ret < 0) - return ret; - ret = ohci_run(ohci); if (ret < 0) { dev_err(hcd->self.controller, "can't start %s\n", @@ -53,6 +54,7 @@ static const struct hc_driver exynos_ohci_hc_driver = { .irq= ohci_irq, .flags = HCD_MEMORY|HCD_USB11, + .reset = ohci_exynos_reset, .start = ohci_exynos_start, .stop = ohci_stop, .shutdown = ohci_shutdown, -- 1.7.7.3 -- 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] x86, fpu: avoid FPU lazy restore after suspend
When a cpu enters S3 state, the FPU state is lost. After resuming for S3, if we try to lazy restore the FPU for a process running on the same CPU, this will result in a corrupted FPU context. We can just invalidate the "fpu_owner_task", so nobody will try to lazy restore a state which no longer exists in the hardware. Tested with a 64-bit kernel on a 4-core Ivybridge CPU with eagerfpu=off, by doing thousands of suspend/resume cycles with 4 processes doing FPU operations running. Without the patch, a process is killed after a few hundreds cycles by a SIGFPE. The issue seems to exist since 3.4 (after the FPU lazy restore was actually implemented), to apply the change to 3.4, "this_cpu_write" needs to be replaced by percpu_write. Cc: Duncan Laurie Cc: Olof Johansson Cc: [v3.4+] # for 3.4 need to replace this_cpu_write by percpu_write Signed-off-by: Vincent Palatin --- arch/x86/kernel/smpboot.c |5 + 1 files changed, 5 insertions(+), 0 deletions(-) diff --git a/arch/x86/kernel/smpboot.c b/arch/x86/kernel/smpboot.c index c80a33b..7610c58 100644 --- a/arch/x86/kernel/smpboot.c +++ b/arch/x86/kernel/smpboot.c @@ -68,6 +68,8 @@ #include #include #include +#include +#include #include #include #include @@ -1230,6 +1232,9 @@ int native_cpu_disable(void) clear_local_APIC(); cpu_disable_common(); + + /* the FPU context will be lost, nobody owns it */ + this_cpu_write(fpu_owner_task, NULL); return 0; } -- 1.7.7.3 -- 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/
issue with x86 FPU state after suspend to ram
Hi, On a 4-core Ivybridge platform, when doing a lot of suspend-to-ram/resume cycles, we were observing processes randomly killed by a SIGFPE. When dumping the FPU registers state on the SIGFPE (usually a floating stack underflow/overflow on a floating point arithmetic operation), the FPU registers looks empty or at least corrupted which was more or less impossible with respect to the disassembled floating point code. After doing more tracing, in the faulty case, the process seems to be keeping FPU ownership over a secondary CPU unplug/re-plug triggered by the suspend. Then it's doing a lazy restore of its FPU context (ie just using the current FPU hardware registers as he is the owner) instead of writing them back to the hardware from the version previously saved in the task context, despite the fact the whole FPU hardware state has been lost. Just invalidating the "fpu_owner_task" when disabling a secondary CPU seems to solve my issue (it's already reset for the primary CPU). By the way, when FPU the lazy restore patch was discussed back in february, Ingo commented (in http://permalink.gmane.org/gmane.linux.kernel/1255423) : " I guess the CPU hotplug case deserves a comment in the code: CPU hotplug + replug of the same (but meanwhile reset) CPU is safe because fpu_owner_task[cpu] gets reset to NULL. " That contradicts my previous observation, so maybe I have totally overlooked something in this mechanism. Can you comment ? I'm still putting my patch proposal in this thread. The issue seems to exist since 3.4 after the FPU lazy restore was actually implemented by commit 7e16838d "i387: support lazy restore of FPU state". But the issue is mainly visible on 3.4 and 3.6 since on tip of tree, it is hidden by the eager fpu implementation for platforms with xsave support, but it still happens with eagerfpu=off. To apply this change to 3.4, "this_cpu_write" needs to be replaced by percpu_write. -- Vincent -- 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 v2] x86, fpu: avoid FPU lazy restore after suspend
When a cpu enters S3 state, the FPU state is lost. After resuming for S3, if we try to lazy restore the FPU for a process running on the same CPU, this will result in a corrupted FPU context. Ensure that "fpu_owner_task" is properly invalided when (re-)initializing a CPU, so nobody will try to lazy restore a state which doesn't exist in the hardware. Tested with a 64-bit kernel on a 4-core Ivybridge CPU with eagerfpu=off, by doing thousands of suspend/resume cycles with 4 processes doing FPU operations running. Without the patch, a process is killed after a few hundreds cycles by a SIGFPE. The issue seems to exist since 3.4 (after the FPU lazy restore was actually implemented), to apply the change to 3.4, "this_cpu_write" needs to be replaced by percpu_write. Cc: Duncan Laurie Cc: Olof Johansson Cc: [v3.4+] # for 3.4 need to replace this_cpu_write by percpu_write Signed-off-by: Vincent Palatin --- arch/x86/include/asm/fpu-internal.h | 15 +-- arch/x86/kernel/smpboot.c |5 + 2 files changed, 14 insertions(+), 6 deletions(-) diff --git a/arch/x86/include/asm/fpu-internal.h b/arch/x86/include/asm/fpu-internal.h index 831dbb9..41ab26e 100644 --- a/arch/x86/include/asm/fpu-internal.h +++ b/arch/x86/include/asm/fpu-internal.h @@ -399,14 +399,17 @@ static inline void drop_init_fpu(struct task_struct *tsk) typedef struct { int preload; } fpu_switch_t; /* - * FIXME! We could do a totally lazy restore, but we need to - * add a per-cpu "this was the task that last touched the FPU - * on this CPU" variable, and the task needs to have a "I last - * touched the FPU on this CPU" and check them. + * Must be run with preemption disabled: this clears the fpu_owner_task, + * on this CPU. * - * We don't do that yet, so "fpu_lazy_restore()" always returns - * false, but some day.. + * This will disable any lazy FPU state restore of the current FPU state, + * but if the current thread owns the FPU, it will still be saved by. */ +static inline void __cpu_disable_lazy_restore(unsigned int cpu) +{ + per_cpu(fpu_owner_task, cpu) = NULL; +} + static inline int fpu_lazy_restore(struct task_struct *new, unsigned int cpu) { return new == this_cpu_read_stable(fpu_owner_task) && diff --git a/arch/x86/kernel/smpboot.c b/arch/x86/kernel/smpboot.c index c80a33b..f3e2ec8 100644 --- a/arch/x86/kernel/smpboot.c +++ b/arch/x86/kernel/smpboot.c @@ -68,6 +68,8 @@ #include #include #include +#include +#include #include #include #include @@ -818,6 +820,9 @@ int __cpuinit native_cpu_up(unsigned int cpu, struct task_struct *tidle) per_cpu(cpu_state, cpu) = CPU_UP_PREPARE; + /* the FPU context is blank, nobody can own it */ + __cpu_disable_lazy_restore(cpu); + err = do_boot_cpu(apicid, cpu, tidle); if (err) { pr_debug("do_boot_cpu failed %d\n", err); -- 1.7.7.3 -- 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] x86, fpu: avoid FPU lazy restore after suspend
When a cpu enters S3 state, the FPU state is lost. After resuming for S3, if we try to lazy restore the FPU for a process running on the same CPU, this will result in a corrupted FPU context. Ensure that "fpu_owner_task" is properly invalided when (re-)initializing a CPU, so nobody will try to lazy restore a state which doesn't exist in the hardware. Tested with a 64-bit kernel on a 4-core Ivybridge CPU with eagerfpu=off, by doing thousands of suspend/resume cycles with 4 processes doing FPU operations running. Without the patch, a process is killed after a few hundreds cycles by a SIGFPE. Cc: Duncan Laurie Cc: Olof Johansson Cc: [v3.4+] # for 3.4 need to replace this_cpu_write by percpu_write Signed-off-by: Vincent Palatin --- Hi, The patch updated according the HPA and Linus comments. I'm still re-running the testing on v3. Change in v3: - remove misleading comment about 3.4 in the description. Change in v2: - add an helper function and comment in fpu-internal.h as described by Linus - do the cleaning in the native_cpu_up function as suggested by HPA Vincent arch/x86/include/asm/fpu-internal.h | 15 +-- arch/x86/kernel/smpboot.c |5 + 2 files changed, 14 insertions(+), 6 deletions(-) diff --git a/arch/x86/include/asm/fpu-internal.h b/arch/x86/include/asm/fpu-internal.h index 831dbb9..41ab26e 100644 --- a/arch/x86/include/asm/fpu-internal.h +++ b/arch/x86/include/asm/fpu-internal.h @@ -399,14 +399,17 @@ static inline void drop_init_fpu(struct task_struct *tsk) typedef struct { int preload; } fpu_switch_t; /* - * FIXME! We could do a totally lazy restore, but we need to - * add a per-cpu "this was the task that last touched the FPU - * on this CPU" variable, and the task needs to have a "I last - * touched the FPU on this CPU" and check them. + * Must be run with preemption disabled: this clears the fpu_owner_task, + * on this CPU. * - * We don't do that yet, so "fpu_lazy_restore()" always returns - * false, but some day.. + * This will disable any lazy FPU state restore of the current FPU state, + * but if the current thread owns the FPU, it will still be saved by. */ +static inline void __cpu_disable_lazy_restore(unsigned int cpu) +{ + per_cpu(fpu_owner_task, cpu) = NULL; +} + static inline int fpu_lazy_restore(struct task_struct *new, unsigned int cpu) { return new == this_cpu_read_stable(fpu_owner_task) && diff --git a/arch/x86/kernel/smpboot.c b/arch/x86/kernel/smpboot.c index c80a33b..f3e2ec8 100644 --- a/arch/x86/kernel/smpboot.c +++ b/arch/x86/kernel/smpboot.c @@ -68,6 +68,8 @@ #include #include #include +#include +#include #include #include #include @@ -818,6 +820,9 @@ int __cpuinit native_cpu_up(unsigned int cpu, struct task_struct *tidle) per_cpu(cpu_state, cpu) = CPU_UP_PREPARE; + /* the FPU context is blank, nobody can own it */ + __cpu_disable_lazy_restore(cpu); + err = do_boot_cpu(apicid, cpu, tidle); if (err) { pr_debug("do_boot_cpu failed %d\n", err); -- 1.7.7.3 -- 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] x86, fpu: avoid FPU lazy restore after suspend
On Fri, Nov 30, 2012 at 11:55 AM, H. Peter Anvin wrote: > > On 11/30/2012 11:54 AM, Vincent Palatin wrote: > >> > > I have done a patch v2 according to your suggestions. > > I will run the testing on it now. > > I probably need at least 2 to 3 hours to validate it. > > > > That would be super. Let me know and I'll queue it up and send a pull > request with this and a few more urgent things to Linus. I have done 1000+ cycles so far with patch v3 (on 4-core Ivybridge and no eagerfpu), and did not hit my issue. I let the testing going on, but wrt the issue after suspend, this fixes it with very high probability (ie I have never done that many cycles without hitting the issue). -- Vincent -- 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] regulator: read low power states configuration from device tree
The regulators state during a system wide low power state can currently only be described in platform data. Add the option to configure them from the device tree. Signed-off-by: Vincent Palatin --- .../devicetree/bindings/regulator/regulator.txt| 6 + drivers/regulator/of_regulator.c | 31 ++ 2 files changed, 37 insertions(+) diff --git a/Documentation/devicetree/bindings/regulator/regulator.txt b/Documentation/devicetree/bindings/regulator/regulator.txt index 48a3b8e..e259b43 100644 --- a/Documentation/devicetree/bindings/regulator/regulator.txt +++ b/Documentation/devicetree/bindings/regulator/regulator.txt @@ -10,6 +10,12 @@ Optional properties: - regulator-always-on: boolean, regulator should never be disabled - regulator-boot-on: bootloader/firmware enabled regulator - regulator-allow-bypass: allow the regulator to go into bypass mode +- regulator-suspend-disk-microvolt: voltage applied when entering S2D +- regulator-suspend-disk-disabled: turn off when entering S2D +- regulator-suspend-mem-microvolt: voltage applied when entering S2M +- regulator-suspend-mem-disabled: turn off when entering S2M +- regulator-suspend-standby-microvolt: voltage applied when entering standby +- regulator-suspend-standby-disabled: turn off when entering standby - -supply: phandle to the parent supply/regulator node - regulator-ramp-delay: ramp delay for regulator(in uV/uS) diff --git a/drivers/regulator/of_regulator.c b/drivers/regulator/of_regulator.c index f3c8f8f..4abc18c 100644 --- a/drivers/regulator/of_regulator.c +++ b/drivers/regulator/of_regulator.c @@ -21,6 +21,7 @@ static void of_get_regulation_constraints(struct device_node *np, { const __be32 *min_uV, *max_uV, *uV_offset; const __be32 *min_uA, *max_uA, *ramp_delay; + const __be32 *state_disk_uV, *state_mem_uV, *state_standby_uV; struct regulation_constraints *constraints = &(*init_data)->constraints; constraints->name = of_get_property(np, "regulator-name", NULL); @@ -67,6 +68,36 @@ static void of_get_regulation_constraints(struct device_node *np, ramp_delay = of_get_property(np, "regulator-ramp-delay", NULL); if (ramp_delay) constraints->ramp_delay = be32_to_cpu(*ramp_delay); + + /* regulator state for suspend to disk */ + state_disk_uV = of_get_property(np, "regulator-suspend-disk-microvolt", + NULL); + if (state_disk_uV) { + constraints->state_disk.uV = be32_to_cpu(*state_disk_uV); + constraints->state_disk.enabled = true; + } + if (of_find_property(np, "regulator-suspend-disk-disabled", NULL)) + constraints->state_disk.disabled = true; + + /* regulator state for suspend to RAM */ + state_mem_uV = of_get_property(np, "regulator-suspend-mem-microvolt", + NULL); + if (state_mem_uV) { + constraints->state_mem.uV = be32_to_cpu(*state_mem_uV); + constraints->state_mem.enabled = true; + } + if (of_find_property(np, "regulator-suspend-mem-disabled", NULL)) + constraints->state_mem.disabled = true; + + /* regulator state for standby */ + state_standby_uV = of_get_property(np, + "regulator-suspend-standby-microvolt", NULL); + if (state_standby_uV) { + constraints->state_standby.uV = be32_to_cpu(*state_standby_uV); + constraints->state_standby.enabled = true; + } + if (of_find_property(np, "regulator-suspend-standby-disabled", NULL)) + constraints->state_standby.disabled = true; } /** -- 1.8.3 -- 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] regulator: read low power states configuration from device tree
On Thu, Jul 25, 2013 at 1:03 PM, Mark Brown wrote: > On Thu, Jul 25, 2013 at 12:42:00PM -0700, Vincent Palatin wrote: > >> +- regulator-suspend-disk-microvolt: voltage applied when entering S2D >> +- regulator-suspend-disk-disabled: turn off when entering S2D >> +- regulator-suspend-mem-microvolt: voltage applied when entering S2M >> +- regulator-suspend-mem-disabled: turn off when entering S2M >> +- regulator-suspend-standby-microvolt: voltage applied when entering standby >> +- regulator-suspend-standby-disabled: turn off when entering standby > > The reason this isn't in device tree at the minute is that suspend to > disk and suspend to RAM are somewhat Linux specific concepts and the > whole thing gets more and more dynamic as time moves forwards with the > suspend state for practical systems depending on the instantaneous > device state prior to entering suspend and the bits that are fixed often > involving sequencing elements and so on which get fixed in hardware > and/or bootloader. Do you have practical systems where this is needed? Yes, on a Chromebook machine, an internal USB device power rail is connected to one of the FET of a TPS65090, the device is leaking power in suspend-to-RAM, it would be nice to cut the FET during suspend. > It's also not clear to me hat the -disabled properties make sense; if we > have properties for the state when enabled I'd expect them to allow > things to be marked as enabled or disabled (with don't touch as the > default). you mean declaring an optional (string) property such as : regulator-suspend-mem-state which can take the value "enabled" or "disabled" e.g. power-regulator { compatible = "ti,tps65090"; reg = <0x48>; voltage-regulators { VFET4 { regulator-name = "usb_leaker"; regulator-suspend-mem-state = "disabled"; }; }; }; -- 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] HID: usbhid: ignore Jabra speakerphones HID interface
Add a quirk to ignore Jabra speakerphone 410 and 510 devices HID interface. On those devices, the USB audio interface is working nicely, but the HID interface is not working with the kernel usbhid driver, and it requires a specific userspace program. We could unbind it from userspace but just attaching the usbhid driver has sometimes nasty effects: either confusing the device state machine or triggering a storm of volume key events making eventual sound UI blinking like crazy. Signed-off-by: Vincent Palatin --- drivers/hid/hid-ids.h | 4 drivers/hid/usbhid/hid-quirks.c | 3 +++ 2 files changed, 7 insertions(+) diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h index 38535c9..533815b 100644 --- a/drivers/hid/hid-ids.h +++ b/drivers/hid/hid-ids.h @@ -447,6 +447,10 @@ #define USB_VENDOR_ID_IRTOUCHSYSTEMS 0x6615 #define USB_DEVICE_ID_IRTOUCH_INFRARED_USB 0x0070 +#define USB_VENDOR_ID_JABRA0x0b0e +#define USB_DEVICE_ID_JABRA_SPEAK_410 0x0412 +#define USB_DEVICE_ID_JABRA_SPEAK_510 0x0420 + #define USB_VENDOR_ID_JESS 0x0c45 #define USB_DEVICE_ID_JESS_YUREX 0x1010 diff --git a/drivers/hid/usbhid/hid-quirks.c b/drivers/hid/usbhid/hid-quirks.c index 19b8360..d8c9aaf 100644 --- a/drivers/hid/usbhid/hid-quirks.c +++ b/drivers/hid/usbhid/hid-quirks.c @@ -109,6 +109,9 @@ static const struct hid_blacklist { { USB_VENDOR_ID_SIGMA_MICRO, USB_DEVICE_ID_SIGMA_MICRO_KEYBOARD, HID_QUIRK_NO_INIT_REPORTS }, { USB_VENDOR_ID_KYE, USB_DEVICE_ID_KYE_MOUSEPEN_I608X, HID_QUIRK_MULTI_INPUT }, { USB_VENDOR_ID_KYE, USB_DEVICE_ID_KYE_EASYPEN_M610X, HID_QUIRK_MULTI_INPUT }, + + { USB_VENDOR_ID_JABRA, USB_DEVICE_ID_JABRA_SPEAK_410, HID_QUIRK_IGNORE }, + { USB_VENDOR_ID_JABRA, USB_DEVICE_ID_JABRA_SPEAK_510, HID_QUIRK_IGNORE }, { 0, 0 } }; -- 1.8.2.1 -- 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] HID: usbhid: ignore Jabra speakerphones HID interface
On Wed, May 22, 2013 at 2:27 PM, Jiri Kosina wrote: > > Please do this in hid_ignore_list[] in drivers/hid/hid-core.c instead. Thanks for the advice ! I will update the patch accordingly. -- Vincent -- 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] HID: usbhid: ignore Jabra speakerphones HID interface
Add a quirk to ignore Jabra speakerphone 410 and 510 devices HID interface. On those devices, the USB audio interface is working nicely, but the HID interface is not working with the kernel usbhid driver, and it requires a specific userspace program. We could unbind it from userspace but just attaching the usbhid driver has sometimes nasty effects: either confusing the device state machine or triggering a storm of volume key events making eventual sound UI blinking like crazy. Signed-off-by: Vincent Palatin --- drivers/hid/hid-core.c | 2 ++ drivers/hid/hid-ids.h | 4 2 files changed, 6 insertions(+) diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c index 264f550..5d2ef66 100644 --- a/drivers/hid/hid-core.c +++ b/drivers/hid/hid-core.c @@ -2042,6 +2042,8 @@ static const struct hid_device_id hid_ignore_list[] = { { HID_USB_DEVICE(USB_VENDOR_ID_GTCO, USB_DEVICE_ID_GTCO_1006) }, { HID_USB_DEVICE(USB_VENDOR_ID_GTCO, USB_DEVICE_ID_GTCO_1007) }, { HID_USB_DEVICE(USB_VENDOR_ID_IMATION, USB_DEVICE_ID_DISC_STAKKA) }, + { HID_USB_DEVICE(USB_VENDOR_ID_JABRA, USB_DEVICE_ID_JABRA_SPEAK_410) }, + { HID_USB_DEVICE(USB_VENDOR_ID_JABRA, USB_DEVICE_ID_JABRA_SPEAK_510) }, { HID_USB_DEVICE(USB_VENDOR_ID_KBGEAR, USB_DEVICE_ID_KBGEAR_JAMSTUDIO) }, { HID_USB_DEVICE(USB_VENDOR_ID_KWORLD, USB_DEVICE_ID_KWORLD_RADIO_FM700) }, { HID_USB_DEVICE(USB_VENDOR_ID_KYE, USB_DEVICE_ID_KYE_GPEN_560) }, diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h index 38535c9..533815b 100644 --- a/drivers/hid/hid-ids.h +++ b/drivers/hid/hid-ids.h @@ -447,6 +447,10 @@ #define USB_VENDOR_ID_IRTOUCHSYSTEMS 0x6615 #define USB_DEVICE_ID_IRTOUCH_INFRARED_USB 0x0070 +#define USB_VENDOR_ID_JABRA0x0b0e +#define USB_DEVICE_ID_JABRA_SPEAK_410 0x0412 +#define USB_DEVICE_ID_JABRA_SPEAK_510 0x0420 + #define USB_VENDOR_ID_JESS 0x0c45 #define USB_DEVICE_ID_JESS_YUREX 0x1010 -- 1.8.2.1 -- 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] rtc: recycle id when unloading a rtc driver
When calling rtc_device_unregister, we are not freeing the id used by the driver. So when doing a unload/load cycle for a RTC driver (e.g. rmmod rtc_cmos && modprobe rtc_cmos), its id is incremented by one. As a consequence, we no longer have neither an rtc0 driver nor a /proc/driver/rtc (as it only exists for the first driver). Signed-off-by: Vincent Palatin --- drivers/rtc/class.c |1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/drivers/rtc/class.c b/drivers/rtc/class.c index dc4c274..37b1d82 100644 --- a/drivers/rtc/class.c +++ b/drivers/rtc/class.c @@ -238,6 +238,7 @@ void rtc_device_unregister(struct rtc_device *rtc) rtc_proc_del_device(rtc); device_unregister(&rtc->dev); rtc->ops = NULL; + ida_simple_remove(&rtc_ida, rtc->id); mutex_unlock(&rtc->ops_lock); put_device(&rtc->dev); } -- 1.7.7.3 -- 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: [PATCHv6 1/3] usb: USB Type-C connector class
Sorry if I'm making redundant comments with previous discussions, I might have missed a few threads. On Mon, Aug 22, 2016 at 2:05 PM, Heikki Krogerus wrote: > The purpose of USB Type-C connector class is to provide > unified interface for the user space to get the status and > basic information about USB Type-C connectors on a system, > control over data role swapping, and when the port supports > USB Power Delivery, also control over power role swapping > and Alternate Modes. > > Signed-off-by: Heikki Krogerus > --- > Documentation/ABI/testing/sysfs-class-typec | 199 + > Documentation/usb/typec.txt | 103 +++ > MAINTAINERS |9 + > drivers/usb/Kconfig |2 + > drivers/usb/Makefile|2 + > drivers/usb/typec/Kconfig |7 + > drivers/usb/typec/Makefile |1 + > drivers/usb/typec/typec.c | 1090 > +++ > include/linux/usb/typec.h | 260 +++ > 9 files changed, 1673 insertions(+) > create mode 100644 Documentation/ABI/testing/sysfs-class-typec > create mode 100644 Documentation/usb/typec.txt > create mode 100644 drivers/usb/typec/Kconfig > create mode 100644 drivers/usb/typec/Makefile > create mode 100644 drivers/usb/typec/typec.c > create mode 100644 include/linux/usb/typec.h > > diff --git a/Documentation/ABI/testing/sysfs-class-typec > b/Documentation/ABI/testing/sysfs-class-typec > new file mode 100644 > index 000..e6179d3 > --- /dev/null > +++ b/Documentation/ABI/testing/sysfs-class-typec > @@ -0,0 +1,199 @@ > +USB Type-C port devices (eg. /sys/class/typec/usbc0/) > + > +What: /sys/class/typec//current_data_role > +Date: June 2016 > +Contact: Heikki Krogerus > +Description: > + The current USB data role the port is operating in. This > + attribute can be used for requesting data role swapping on the > + port. role swapping is sometimes a long operation. maybe we need to say explicitly whether the 'write' is synchronous and returns when the swap has succeeded / failed or asynchronous (and requires polling current_data_role afterwards to know the result ?) > + > + Valid values: > + - host > + - device the USB workgroup has settled for DFP/UFP rather than host/device ? > + > +What: /sys/class/typec//current_power_role > +Date: June 2016 > +Contact: Heikki Krogerus > +Description: > + The current power role of the port. This attribute can be used > + to request power role swap on the port when the port supports ditto > + USB Power Delivery. > + > + Valid values: > + - source > + - sink > + > +What: /sys/class/typec//current_vconn_role > +Date: June 2016 > +Contact: Heikki Krogerus > +Description: > + Shows the current VCONN role of the port. This attribute can > be > + used to request VCONN role swap on the port when the port > + supports USB Power Delivery. > + > + Valid values are: > + - source > + - sink either we are currently sourcing vconn or not, but even if you are not, you are probably not a vconn sink either (ie only vconn-powered accessory are, your usual linux-powered laptop/phone is probably not) > + > +What: /sys/class/typec//power_operation_mode > +Date: June 2016 > +Contact: Heikki Krogerus > +Description: > + Shows the current power operational mode the port is in. > + > + Valid values: > + - USB - Normal power levels defined in USB specifications > + - BC1.2 - Power levels defined in Battery Charging > Specification > + v1.2 > + - USB Type-C 1.5A - Higher 1.5A current defined in USB Type-C > + specification. > + - USB Type-C 3.0A - Higher 3A current defined in USB Type-C > + specification. > +- USB Power Delivery - The voltages and currents defined in > USB > + Power Delivery specification > + > +What: /sys/class/typec//preferred_role > +Date: June 2016 > +Contact: Heikki Krogerus > +Description: > + The user space can notify the driver about the preferred role. > + It should be handled as enabling of Try.SRC or Try.SNK, as > + defined in USB Type-C specification, in the port drivers. By > + default there is no preferred role. > + > + Valid values: > + - host > + - device > + - For example "none" to remove preference (anything else > except >
Re: [PATCHv6 1/3] usb: USB Type-C connector class
On Thu, Aug 25, 2016 at 1:59 PM, Heikki Krogerus wrote: > Hi, > > On Wed, Aug 24, 2016 at 04:08:23PM +0200, Vincent Palatin wrote: >> Sorry if I'm making redundant comments with previous discussions, I >> might have missed a few threads. >> >> >> On Mon, Aug 22, 2016 at 2:05 PM, Heikki Krogerus >> wrote: >> > The purpose of USB Type-C connector class is to provide >> > unified interface for the user space to get the status and >> > basic information about USB Type-C connectors on a system, >> > control over data role swapping, and when the port supports >> > USB Power Delivery, also control over power role swapping >> > and Alternate Modes. >> > >> > Signed-off-by: Heikki Krogerus >> > --- >> > Documentation/ABI/testing/sysfs-class-typec | 199 + >> > Documentation/usb/typec.txt | 103 +++ >> > MAINTAINERS |9 + >> > drivers/usb/Kconfig |2 + >> > drivers/usb/Makefile|2 + >> > drivers/usb/typec/Kconfig |7 + >> > drivers/usb/typec/Makefile |1 + >> > drivers/usb/typec/typec.c | 1090 >> > +++ >> > include/linux/usb/typec.h | 260 +++ >> > 9 files changed, 1673 insertions(+) >> > create mode 100644 Documentation/ABI/testing/sysfs-class-typec >> > create mode 100644 Documentation/usb/typec.txt >> > create mode 100644 drivers/usb/typec/Kconfig >> > create mode 100644 drivers/usb/typec/Makefile >> > create mode 100644 drivers/usb/typec/typec.c >> > create mode 100644 include/linux/usb/typec.h >> > >> > diff --git a/Documentation/ABI/testing/sysfs-class-typec >> > b/Documentation/ABI/testing/sysfs-class-typec >> > new file mode 100644 >> > index 000..e6179d3 >> > --- /dev/null >> > +++ b/Documentation/ABI/testing/sysfs-class-typec >> > @@ -0,0 +1,199 @@ >> > +USB Type-C port devices (eg. /sys/class/typec/usbc0/) >> > + >> > +What: /sys/class/typec//current_data_role >> > +Date: June 2016 >> > +Contact: Heikki Krogerus >> > +Description: >> > + The current USB data role the port is operating in. This >> > + attribute can be used for requesting data role swapping on >> > the >> > + port. >> >> role swapping is sometimes a long operation. maybe we need to say >> explicitly whether the 'write' is synchronous and returns when the >> swap has succeeded / failed or asynchronous (and requires polling >> current_data_role afterwards to know the result ?) > > OK. > >> > + >> > + Valid values: >> > + - host >> > + - device >> >> the USB workgroup has settled for DFP/UFP rather than host/device ? > > (I don't think the workgroup has settled on anything.) > > I already proposed DFP/UFP, but it did not fly. The naming really does > not need to reflect the spec exactly, especially since there is no > guarantee they will not change the names in future versions of the > spec. "host/device", "source/sink" are completely understandable, > unlike DRP/UFP. OK. >> >> > + >> > +What: /sys/class/typec//current_power_role >> > +Date: June 2016 >> > +Contact: Heikki Krogerus >> > +Description: >> > + The current power role of the port. This attribute can be >> > used >> > + to request power role swap on the port when the port >> > supports >> >> ditto >> >> >> > + USB Power Delivery. >> > + >> > + Valid values: >> > + - source >> > + - sink >> > + >> > +What: /sys/class/typec//current_vconn_role >> > +Date: June 2016 >> > +Contact: Heikki Krogerus >> > +Description: >> > + Shows the current VCONN role of the port. This attribute >> > can be >> > + used to request VCONN role swap on the port when the port >> > + supports USB Power Delivery. >> > + >> > + Valid values are: >> > + - source >> > + - sink >> >> >
[PATCH] usb: Add device quirk for Logitech PTZ cameras
Add a device quirk for the Logitech PTZ Pro Camera and its sibling the ConferenceCam CC3000e Camera. This fixes the failed camera enumeration on some boot, particularly on machines with fast CPU. Tested by connecting a Logitech PTZ Pro Camera to a machine with a Haswell Core i7-4600U CPU @ 2.10GHz, and doing thousands of reboot cycles while recording the kernel logs and taking camera picture after each boot. Before the patch, more than 7% of the boots show some enumeration transfer failures and in a few of them, the kernel is giving up before actually enumerating the webcam. After the patch, the enumeration has been correct on every reboot. Signed-off-by: Vincent Palatin --- drivers/usb/core/quirks.c | 7 +++ 1 file changed, 7 insertions(+) diff --git a/drivers/usb/core/quirks.c b/drivers/usb/core/quirks.c index d85abfe..0a56de7 100644 --- a/drivers/usb/core/quirks.c +++ b/drivers/usb/core/quirks.c @@ -54,6 +54,13 @@ static const struct usb_device_id usb_quirk_list[] = { { USB_DEVICE(0x046d, 0x082d), .driver_info = USB_QUIRK_DELAY_INIT }, { USB_DEVICE(0x046d, 0x0843), .driver_info = USB_QUIRK_DELAY_INIT }, + /* Logitech ConferenceCam CC3000e */ + { USB_DEVICE(0x046d, 0x0847), .driver_info = USB_QUIRK_DELAY_INIT }, + { USB_DEVICE(0x046d, 0x0848), .driver_info = USB_QUIRK_DELAY_INIT }, + + /* Logitech PTZ Pro Camera */ + { USB_DEVICE(0x046d, 0x0853), .driver_info = USB_QUIRK_DELAY_INIT }, + /* Logitech Quickcam Fusion */ { USB_DEVICE(0x046d, 0x08c1), .driver_info = USB_QUIRK_RESET_RESUME }, -- 2.6.0.rc2.230.g3dd15c0 -- 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] USB: quirks: ignore remote wake-up on Fibocom L850-GL LTE modem
This LTE modem (M.2 card) has a bug in its power managment: there is some kind of race condition for U3 wake-up between the host and the device. The modem firmware sometimes crashes/locks when both events happen at the same time and the modem fully drops off the USB bus (and sometimes re-enumerates, sometimes just gets stuck until the next reboot). Tested with the modem wired to the XHCI controller on an AMD 3015Ce platform. Without the patch, the modem dropped of the USB bus 5 times in 3 days. With the quirk, it stayed connected for a week while the 'runtime_suspended_time' counter incremented as excepted. --- drivers/usb/core/quirks.c | 4 1 file changed, 4 insertions(+) diff --git a/drivers/usb/core/quirks.c b/drivers/usb/core/quirks.c index 6ade3daf7858..76ac5d6555ae 100644 --- a/drivers/usb/core/quirks.c +++ b/drivers/usb/core/quirks.c @@ -498,6 +498,10 @@ static const struct usb_device_id usb_quirk_list[] = { /* DJI CineSSD */ { USB_DEVICE(0x2ca3, 0x0031), .driver_info = USB_QUIRK_NO_LPM }, + /* Fibocom L850-GL LTE Modem */ + { USB_DEVICE(0x2cb7, 0x0007), .driver_info = + USB_QUIRK_IGNORE_REMOTE_WAKEUP }, + /* INTEL VALUE SSD */ { USB_DEVICE(0x8086, 0xf1a5), .driver_info = USB_QUIRK_RESET_RESUME }, -- 2.31.0.rc2.261.g7f71774620-goog
Re: [PATCH] USB: quirks: ignore remote wake-up on Fibocom L850-GL LTE modem
On Fri, Mar 19, 2021 at 1:41 PM Vincent Palatin wrote: > > This LTE modem (M.2 card) has a bug in its power managment: > there is some kind of race condition for U3 wake-up between the host and > the device. The modem firmware sometimes crashes/locks when both events > happen at the same time and the modem fully drops off the USB bus (and > sometimes re-enumerates, sometimes just gets stuck until the next > reboot). > > Tested with the modem wired to the XHCI controller on an AMD 3015Ce > platform. Without the patch, the modem dropped of the USB bus 5 times in > 3 days. With the quirk, it stayed connected for a week while the > 'runtime_suspended_time' counter incremented as excepted. Forgot the sign-off-by I will re-send > --- > drivers/usb/core/quirks.c | 4 > 1 file changed, 4 insertions(+) > > diff --git a/drivers/usb/core/quirks.c b/drivers/usb/core/quirks.c > index 6ade3daf7858..76ac5d6555ae 100644 > --- a/drivers/usb/core/quirks.c > +++ b/drivers/usb/core/quirks.c > @@ -498,6 +498,10 @@ static const struct usb_device_id usb_quirk_list[] = { > /* DJI CineSSD */ > { USB_DEVICE(0x2ca3, 0x0031), .driver_info = USB_QUIRK_NO_LPM }, > > + /* Fibocom L850-GL LTE Modem */ > + { USB_DEVICE(0x2cb7, 0x0007), .driver_info = > + USB_QUIRK_IGNORE_REMOTE_WAKEUP }, > + > /* INTEL VALUE SSD */ > { USB_DEVICE(0x8086, 0xf1a5), .driver_info = USB_QUIRK_RESET_RESUME }, > > -- > 2.31.0.rc2.261.g7f71774620-goog >
[PATCHi v2] USB: quirks: ignore remote wake-up on Fibocom L850-GL LTE modem
This LTE modem (M.2 card) has a bug in its power management: there is some kind of race condition for U3 wake-up between the host and the device. The modem firmware sometimes crashes/locks when both events happen at the same time and the modem fully drops off the USB bus (and sometimes re-enumerates, sometimes just gets stuck until the next reboot). Tested with the modem wired to the XHCI controller on an AMD 3015Ce platform. Without the patch, the modem dropped of the USB bus 5 times in 3 days. With the quirk, it stayed connected for a week while the 'runtime_suspended_time' counter incremented as excepted. Signed-off-by: Vincent Palatin --- drivers/usb/core/quirks.c | 4 1 file changed, 4 insertions(+) diff --git a/drivers/usb/core/quirks.c b/drivers/usb/core/quirks.c index 6ade3daf7858..76ac5d6555ae 100644 --- a/drivers/usb/core/quirks.c +++ b/drivers/usb/core/quirks.c @@ -498,6 +498,10 @@ static const struct usb_device_id usb_quirk_list[] = { /* DJI CineSSD */ { USB_DEVICE(0x2ca3, 0x0031), .driver_info = USB_QUIRK_NO_LPM }, + /* Fibocom L850-GL LTE Modem */ + { USB_DEVICE(0x2cb7, 0x0007), .driver_info = + USB_QUIRK_IGNORE_REMOTE_WAKEUP }, + /* INTEL VALUE SSD */ { USB_DEVICE(0x8086, 0xf1a5), .driver_info = USB_QUIRK_RESET_RESUME }, -- 2.31.0.rc2.261.g7f71774620-goog
Re: [PATCH] usb: serial: option: add Fibocom NL668 variants
On Fri, Nov 20, 2020 at 10:01 AM wrote: > > From: Vincent Palatin > > Update the USB serial option driver support for the Fibocom NL668 Cat.4 > LTE modules as there are actually several different variants. > Got clarifications from Fibocom, there are distinct products: > - VID:PID 1508:1001, NL668 for IOT (no MBIM interface) > - VID:PID 2cb7:01a0, NL668-AM and NL652-EU are laptop M.2 cards (with > MBIM interfaces for Windows/Linux/Chrome OS), respectively for Americas > and Europe. > > usb-devices output for the laptop M.2 cards: > T: Bus=01 Lev=01 Prnt=01 Port=00 Cnt=01 Dev#= 4 Spd=480 MxCh= 0 > D: Ver= 2.00 Cls=ef(misc ) Sub=00 Prot=00 MxPS=64 #Cfgs= 1 > P: Vendor=2cb7 ProdID=01a0 Rev=03.18 > S: Manufacturer=Fibocom Wireless Inc. > S: Product=Fibocom NL652-EU Modem > S: SerialNumber=0123456789ABCDEF > C: #Ifs= 5 Cfg#= 1 Atr=a0 MxPwr=500mA > I: If#= 0 Alt= 0 #EPs= 1 Cls=02(commc) Sub=0e Prot=00 Driver=cdc_mbim > I: If#= 1 Alt= 1 #EPs= 2 Cls=0a(data ) Sub=00 Prot=02 Driver=cdc_mbim > I: If#= 2 Alt= 0 #EPs= 3 Cls=ff(vend.) Sub=00 Prot=00 Driver=(none) > I: If#= 3 Alt= 0 #EPs= 3 Cls=ff(vend.) Sub=00 Prot=00 Driver=(none) > I: If#= 4 Alt= 0 #EPs= 2 Cls=ff(vend.) Sub=ff Prot=ff Driver=(none) > > Signed-off-by: Vincent Palatin > --- > drivers/usb/serial/option.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/drivers/usb/serial/option.c b/drivers/usb/serial/option.c > index 54ca85cc920d..2a6d59bc0201 100644 > --- a/drivers/usb/serial/option.c > +++ b/drivers/usb/serial/option.c > @@ -2046,12 +2046,13 @@ static const struct usb_device_id option_ids[] = { > .driver_info = RSVD(0) | RSVD(1) | RSVD(6) }, > { USB_DEVICE(0x0489, 0xe0b5), > /* Foxconn T77W968 ESIM */ > .driver_info = RSVD(0) | RSVD(1) | RSVD(6) }, > - { USB_DEVICE(0x1508, 0x1001), > /* Fibocom NL668 */ > + { USB_DEVICE(0x1508, 0x1001), > /* Fibocom NL668 (IOT version) */ > .driver_info = RSVD(4) | RSVD(5) | RSVD(6) }, > { USB_DEVICE(0x2cb7, 0x0104), > /* Fibocom NL678 series */ > .driver_info = RSVD(4) | RSVD(5) }, > { USB_DEVICE_INTERFACE_CLASS(0x2cb7, 0x0105, 0xff), > /* Fibocom NL678 series */ > .driver_info = RSVD(6) }, > + { USB_DEVICE(0x2cb7, 0x01a0, 0xff) }, > /* Fibocom NL668-AM/NL652-EU (laptop MBIM) */ I obviously screw up here. Wrong version of the patch with the wrong macro name I will send the updated one. > > { USB_DEVICE_INTERFACE_CLASS(0x305a, 0x1404, 0xff) }, > /* GosunCn GM500 RNDIS */ > { USB_DEVICE_INTERFACE_CLASS(0x305a, 0x1405, 0xff) }, > /* GosunCn GM500 MBIM */ > { USB_DEVICE_INTERFACE_CLASS(0x305a, 0x1406, 0xff) }, > /* GosunCn GM500 ECM/NCM */ > -- > 2.26.2 >
Re: [PATCH v4 4/4] ARM: dts: Add exynos5250-spring device tree
Always a bit late to the game. One small comment inline. Reviewed-by: Vincent Palatin On Thu, Jul 31, 2014 at 9:08 AM, Andreas Färber wrote: > Adds initial support for the HP Chromebook 11. > > Cc: Vincent Palatin > Cc: Doug Anderson > Cc: Stephan van Schaik > Signed-off-by: Andreas Färber > --- > v3 -> v4: > * Fixed samsung,pin-function 1 -> 0 for dp-hpd-gpio > * Replaced dp-hpd-gpio with existing dp_hpd, overriding it > > v2 -> v3: > * Use GPIO_ACTIVE_{LOW,HIGH} (Doug Anderson) > * Use symbolic KEY_POWER instead of comment > * Moved hsic_reset to new USB3503 node's reset-gpios (Vincent Palatin) > * Use dp_hpd_gpio for dp-controller (Doug Anderson, Ajay Kumar) > * Override sd1_{clk,cmd,cd,bus4} pinctrl similar to Snow (Doug Anderson) > * Added ec_irq pinctrl for cros_ec (Doug Anderson) > * Reordered nodes to minimize diff against Snow (Doug Anderson) > * Dropped obsolete mmc_2 override (Doug Anderson) > * Added lid-switch node (Doug Anderson) > * Added gpio-keys pinctrl (Doug Anderson) > * Added bootargs to avoid empty /chosen node and to document console setting > * Renamed s5m8767_pmic node to avoid underscore > * Use new style for overriding inherited pinctrl nodes, too > * Enable i2s0 node > > v1 -> v2: > * Use label-based overriding/extension of nodes. (Doug Anderson) > * Dropped tps65090 for now, until we know where to place it. > * Dropped non-Spring nodes from -cros-common.dtsi rather than disabling them. > * Enabled a missing MMC node for access to internal storage. > * Dropped display-timings from dp-controller node. (Ajay Kumar) > > arch/arm/boot/dts/Makefile | 1 + > arch/arm/boot/dts/exynos5250-spring.dts | 539 > > 2 files changed, 540 insertions(+) > create mode 100644 arch/arm/boot/dts/exynos5250-spring.dts > > diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile > index 80a781f76e88..dec4c292f63d 100644 > --- a/arch/arm/boot/dts/Makefile > +++ b/arch/arm/boot/dts/Makefile > @@ -76,6 +76,7 @@ dtb-$(CONFIG_ARCH_EXYNOS) += exynos4210-origen.dtb \ > exynos5250-arndale.dtb \ > exynos5250-smdk5250.dtb \ > exynos5250-snow.dtb \ > + exynos5250-spring.dtb \ > exynos5260-xyref5260.dtb \ > exynos5410-smdk5410.dtb \ > exynos5420-arndale-octa.dtb \ > diff --git a/arch/arm/boot/dts/exynos5250-spring.dts > b/arch/arm/boot/dts/exynos5250-spring.dts > new file mode 100644 > index ..1fdfa04182fc > --- /dev/null > +++ b/arch/arm/boot/dts/exynos5250-spring.dts > @@ -0,0 +1,539 @@ > +/* > + * Google Spring board device tree source > + * > + * Copyright (c) 2013 Google, Inc > + * Copyright (c) 2014 SUSE LINUX Products GmbH > + * > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU General Public License version 2 as > + * published by the Free Software Foundation. > + */ > + > +/dts-v1/; > +#include > +#include > +#include "exynos5250.dtsi" > + > +/ { > + model = "Google Spring"; > + compatible = "google,spring", "samsung,exynos5250", "samsung,exynos5"; > + > + memory { > + reg = <0x4000 0x8000>; > + }; > + > + chosen { > + bootargs = "console=tty1"; > + }; > + > + gpio-keys { > + compatible = "gpio-keys"; > + pinctrl-names = "default"; > + pinctrl-0 = <&power_key_irq>, <&lid_irq>; > + > + power { > + label = "Power"; > + gpios = <&gpx1 3 GPIO_ACTIVE_LOW>; > + linux,code = ; > + gpio-key,wakeup; > + }; > + > + lid-switch { > + label = "Lid"; > + gpios = <&gpx3 5 GPIO_ACTIVE_LOW>; > + linux,input-type = <5>; /* EV_SW */ > + linux,code = <0>; /* SW_LID */ > + debounce-interval = <1>; > + gpio-key,wakeup; > + }; > + }; > + > + usb3_vbus_reg: regulator-usb3 { > + compatible = "regulator-fixed"; > + regulator-name = "P5.0V_USB3CON"; > + regulator-min-microvolt = <500>; > + regulator-max-microvolt = <500>; > + gpio = <&gpe1 0 GPIO_ACTIVE_LOW>; > +
Re: [PATCH 1/2] [media] V4L: Add camera pan/tilt speed controls
ping ... Any opinion on adding those new controls ? since re-using the existing relative ones was seen as too twisted. Thanks, -- Vincent On Tue, Jul 8, 2014 at 4:49 PM, Vincent Palatin wrote: > The V4L2_CID_PAN_SPEED and V4L2_CID_TILT_SPEED controls allow to move the > camera by setting its rotation speed around its axis. > > Signed-off-by: Vincent Palatin > --- > Documentation/DocBook/media/v4l/compat.xml | 10 ++ > Documentation/DocBook/media/v4l/controls.xml | 21 + > drivers/media/v4l2-core/v4l2-ctrls.c | 2 ++ > include/uapi/linux/v4l2-controls.h | 2 ++ > 4 files changed, 35 insertions(+) > > diff --git a/Documentation/DocBook/media/v4l/compat.xml > b/Documentation/DocBook/media/v4l/compat.xml > index eee6f0f..21910e9 100644 > --- a/Documentation/DocBook/media/v4l/compat.xml > +++ b/Documentation/DocBook/media/v4l/compat.xml > @@ -2545,6 +2545,16 @@ fields changed from _s32 to _u32. > > > > + > + V4L2 in Linux 3.17 > + > + > + Added V4L2_CID_PAN_SPEED and > + V4L2_CID_TILT_SPEED camera controls. > + > + > + > + > >Relation of V4L2 to other Linux multimedia APIs > > diff --git a/Documentation/DocBook/media/v4l/controls.xml > b/Documentation/DocBook/media/v4l/controls.xml > index 47198ee..cdf97f0 100644 > --- a/Documentation/DocBook/media/v4l/controls.xml > +++ b/Documentation/DocBook/media/v4l/controls.xml > @@ -3914,6 +3914,27 @@ by exposure, white balance or focus controls. > > > > + > +spanname="id">V4L2_CID_PAN_SPEED > + integer > + This control turns the > +camera horizontally at the specific speed. The unit is undefined. A > +positive value moves the camera to the right (clockwise when viewed > +from above), a negative value to the left. A value of zero does not > +cause or stop the motion. > + > + > + > + > +spanname="id">V4L2_CID_TILT_SPEED > + integer > + This control turns the > +camera vertically at the specified speed. The unit is undefined. A > +positive value moves the camera up, a negative value down. A value of > +zero does not cause or stop the motion. > + > + > + > > > > diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c > b/drivers/media/v4l2-core/v4l2-ctrls.c > index 55c6832..57ddaf4 100644 > --- a/drivers/media/v4l2-core/v4l2-ctrls.c > +++ b/drivers/media/v4l2-core/v4l2-ctrls.c > @@ -787,6 +787,8 @@ const char *v4l2_ctrl_get_name(u32 id) > case V4L2_CID_AUTO_FOCUS_STOP: return "Auto Focus, Stop"; > case V4L2_CID_AUTO_FOCUS_STATUS:return "Auto Focus, Status"; > case V4L2_CID_AUTO_FOCUS_RANGE: return "Auto Focus, Range"; > + case V4L2_CID_PAN_SPEED:return "Pan, Speed"; > + case V4L2_CID_TILT_SPEED: return "Tilt, Speed"; > > /* FM Radio Modulator control */ > /* Keep the order of the 'case's the same as in videodev2.h! */ > diff --git a/include/uapi/linux/v4l2-controls.h > b/include/uapi/linux/v4l2-controls.h > index 2ac5597..5576044 100644 > --- a/include/uapi/linux/v4l2-controls.h > +++ b/include/uapi/linux/v4l2-controls.h > @@ -745,6 +745,8 @@ enum v4l2_auto_focus_range { > V4L2_AUTO_FOCUS_RANGE_INFINITY = 3, > }; > > +#define V4L2_CID_PAN_SPEED > (V4L2_CID_CAMERA_CLASS_BASE+32) > +#define V4L2_CID_TILT_SPEED > (V4L2_CID_CAMERA_CLASS_BASE+33) > > /* FM Modulator class control IDs */ > > -- > 2.0.0.526.g5318336 > -- 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 4/4] ARM: dts: Add exynos5250-spring device tree
On Thu, Jul 31, 2014 at 10:14 AM, Andreas Färber wrote: > Hi, > > Am 31.07.2014 19:00, schrieb Vincent Palatin: >> Always a bit late to the game. >> One small comment inline. >> >> Reviewed-by: Vincent Palatin > > Thanks, > >> >> On Thu, Jul 31, 2014 at 9:08 AM, Andreas Färber wrote: >>> + usb3_vbus_reg: regulator-usb3 { >>> + compatible = "regulator-fixed"; >>> + regulator-name = "P5.0V_USB3CON"; >>> + regulator-min-microvolt = <500>; >>> + regulator-max-microvolt = <500>; >>> + gpio = <&gpe1 0 GPIO_ACTIVE_LOW>; >>> + enable-active-high; >>> + }; >> >> GPE1_0 GPIO is the HSIC hub (SMSC 3503) reset# line (already defined >> below afaik). > > Yes, that was a suggestion you made on v1. > >> On this design there is no external USB3 port, so no VBUS reg/load >> switch for USB3. > > Could you be a little clearer? Are you suggesting to drop the gpio > property? I just re-tested that without the regulator node plus the > vbus-supply below I don't get any USB2 (so maybe rename the regulator?). The 3503 PHY driver is not fully correct, so we probably need to keep this to get the right init timings when the bootloader has initiliazed it before. but yes renaming the regulator to mention that's actually the hsic hub reset would make it clearer. >>> + >>> + usb@1211 { >>> + samsung,vbus-gpio = <&gpx1 1 GPIO_ACTIVE_HIGH>; >>> + }; >>> + >>> + usb-hub { >>> + compatible = "smsc,usb3503a"; >>> + reset-gpios = <&hsic_reset>; >>> + }; > [...] >>> +&usbdrd_phy { >>> + vbus-supply = <&usb3_vbus_reg>; >>> +}; >>> + >>> +#include "cros-ec-keyboard.dtsi" >>> -- >>> 1.9.3 > > -- > SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany > GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg -- 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] V4L: uvcvideo: Add support for relative pan/tilt controls
On Wed, Jun 25, 2014 at 2:54 AM, Laurent Pinchart wrote: > Hi Pawel, > > On Wednesday 25 June 2014 11:46:24 Pawel Osciak wrote: >> On Tue, Jun 17, 2014 at 11:45 PM, Vincent Palatin wrote: >> > Map V4L2_CID_TILT_RELATIVE and V4L2_CID_PAN_RELATIVE to the standard UVC >> > CT_PANTILT_RELATIVE_CONTROL terminal control request. >> > >> > Tested by plugging a Logitech ConferenceCam C3000e USB camera >> > and controlling pan/tilt from the userspace using the VIDIOC_S_CTRL ioctl. >> > Verified that it can pan and tilt at the same time in both directions. >> > >> > Signed-off-by: Vincent Palatin >> > >> > Change-Id: I7b70b228e5c0126683f5f0be34ffd2807f5783dc >> > --- >> > >> > Changes >> > v2: fix control request name in description. >> >> The patch looks good, but I have a more general comment for everyone to >> consider. This doesn't match the expected functionality of >> controls V4L2_CID_PAN/TILT_RELATIVE. This is basically an on/off switch for >> pan/tilt, which once enabled will keep going until turned off (or I'm >> guessing until the maximum pan/tilt is reached), while the controls are >> supposed to expose an ability to turn the camera by a specified amount. >> Here the amount will also be ignored... > > I agree with you here, and this mismatch between the V4L and UVC controls is > the reason why I haven't implemented relative pan/tilt support. > >> Given that this is a standard UVC control, perhaps we need new V4L2 >> controls for it, as I'm assuming we can't change the meaning of existing >> controls? > > We could extend the meaning of the controls to cover the UVC behaviour in a > device-specific fashion, but that would be confusing for applications, so new > controls might be a better idea. Ok, I will add another patch to create new V4L2_CID_PAN_SPEED / V4L2_CID_TILT_SPEED controls. -- Vincent -- 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] V4L: uvcvideo: Add support for pan/tilt speed controls
Map V4L2_CID_TILT_SPEED and V4L2_CID_PAN_SPEED to the standard UVC CT_PANTILT_RELATIVE_CONTROL terminal control request. Tested by plugging a Logitech ConferenceCam C3000e USB camera and controlling pan/tilt from the userspace using the VIDIOC_S_CTRL ioctl. Verified that it can pan and tilt at the same time in both directions. Signed-off-by: Vincent Palatin Change-Id: I7b70b228e5c0126683f5f0be34ffd2807f5783dc --- drivers/media/usb/uvc/uvc_ctrl.c | 58 +--- 1 file changed, 55 insertions(+), 3 deletions(-) diff --git a/drivers/media/usb/uvc/uvc_ctrl.c b/drivers/media/usb/uvc/uvc_ctrl.c index 0eb82106..d703cb0 100644 --- a/drivers/media/usb/uvc/uvc_ctrl.c +++ b/drivers/media/usb/uvc/uvc_ctrl.c @@ -309,9 +309,8 @@ static struct uvc_control_info uvc_ctrls[] = { .selector = UVC_CT_PANTILT_RELATIVE_CONTROL, .index = 12, .size = 4, - .flags = UVC_CTRL_FLAG_SET_CUR | UVC_CTRL_FLAG_GET_MIN - | UVC_CTRL_FLAG_GET_MAX | UVC_CTRL_FLAG_GET_RES - | UVC_CTRL_FLAG_GET_DEF + .flags = UVC_CTRL_FLAG_SET_CUR + | UVC_CTRL_FLAG_GET_RANGE | UVC_CTRL_FLAG_AUTO_UPDATE, }, { @@ -391,6 +390,35 @@ static void uvc_ctrl_set_zoom(struct uvc_control_mapping *mapping, data[2] = min((int)abs(value), 0xff); } +static __s32 uvc_ctrl_get_rel_speed(struct uvc_control_mapping *mapping, + __u8 query, const __u8 *data) +{ + int first = mapping->offset / 8; + __s8 rel = (__s8)data[first]; + + switch (query) { + case UVC_GET_CUR: + return (rel == 0) ? 0 : (rel > 0 ? data[first+1] +: -data[first+1]); + case UVC_GET_MIN: + return -data[first+1]; + case UVC_GET_MAX: + case UVC_GET_RES: + case UVC_GET_DEF: + default: + return data[first+1]; + } +} + +static void uvc_ctrl_set_rel_speed(struct uvc_control_mapping *mapping, + __s32 value, __u8 *data) +{ + int first = mapping->offset / 8; + + data[first] = value == 0 ? 0 : (value > 0) ? 1 : 0xff; + data[first+1] = min_t(int, abs(value), 0xff); +} + static struct uvc_control_mapping uvc_ctrl_mappings[] = { { .id = V4L2_CID_BRIGHTNESS, @@ -677,6 +705,30 @@ static struct uvc_control_mapping uvc_ctrl_mappings[] = { .data_type = UVC_CTRL_DATA_TYPE_SIGNED, }, { + .id = V4L2_CID_PAN_SPEED, + .name = "Pan (Speed)", + .entity = UVC_GUID_UVC_CAMERA, + .selector = UVC_CT_PANTILT_RELATIVE_CONTROL, + .size = 16, + .offset = 0, + .v4l2_type = V4L2_CTRL_TYPE_INTEGER, + .data_type = UVC_CTRL_DATA_TYPE_SIGNED, + .get= uvc_ctrl_get_rel_speed, + .set= uvc_ctrl_set_rel_speed, + }, + { + .id = V4L2_CID_TILT_SPEED, + .name = "Tilt (Speed)", + .entity = UVC_GUID_UVC_CAMERA, + .selector = UVC_CT_PANTILT_RELATIVE_CONTROL, + .size = 16, + .offset = 16, + .v4l2_type = V4L2_CTRL_TYPE_INTEGER, + .data_type = UVC_CTRL_DATA_TYPE_SIGNED, + .get= uvc_ctrl_get_rel_speed, + .set= uvc_ctrl_set_rel_speed, + }, + { .id = V4L2_CID_PRIVACY, .name = "Privacy", .entity = UVC_GUID_UVC_CAMERA, -- 2.0.0.526.g5318336 -- 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] [media] V4L: Add camera pan/tilt speed controls
The V4L2_CID_PAN_SPEED and V4L2_CID_TILT_SPEED controls allow to move the camera by setting its rotation speed around its axis. Signed-off-by: Vincent Palatin --- Documentation/DocBook/media/v4l/compat.xml | 10 ++ Documentation/DocBook/media/v4l/controls.xml | 21 + drivers/media/v4l2-core/v4l2-ctrls.c | 2 ++ include/uapi/linux/v4l2-controls.h | 2 ++ 4 files changed, 35 insertions(+) diff --git a/Documentation/DocBook/media/v4l/compat.xml b/Documentation/DocBook/media/v4l/compat.xml index eee6f0f..21910e9 100644 --- a/Documentation/DocBook/media/v4l/compat.xml +++ b/Documentation/DocBook/media/v4l/compat.xml @@ -2545,6 +2545,16 @@ fields changed from _s32 to _u32. + + V4L2 in Linux 3.17 + + + Added V4L2_CID_PAN_SPEED and + V4L2_CID_TILT_SPEED camera controls. + + + + Relation of V4L2 to other Linux multimedia APIs diff --git a/Documentation/DocBook/media/v4l/controls.xml b/Documentation/DocBook/media/v4l/controls.xml index 47198ee..cdf97f0 100644 --- a/Documentation/DocBook/media/v4l/controls.xml +++ b/Documentation/DocBook/media/v4l/controls.xml @@ -3914,6 +3914,27 @@ by exposure, white balance or focus controls. + + V4L2_CID_PAN_SPEED + integer + This control turns the +camera horizontally at the specific speed. The unit is undefined. A +positive value moves the camera to the right (clockwise when viewed +from above), a negative value to the left. A value of zero does not +cause or stop the motion. + + + + + V4L2_CID_TILT_SPEED + integer + This control turns the +camera vertically at the specified speed. The unit is undefined. A +positive value moves the camera up, a negative value down. A value of +zero does not cause or stop the motion. + + + diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c b/drivers/media/v4l2-core/v4l2-ctrls.c index 55c6832..57ddaf4 100644 --- a/drivers/media/v4l2-core/v4l2-ctrls.c +++ b/drivers/media/v4l2-core/v4l2-ctrls.c @@ -787,6 +787,8 @@ const char *v4l2_ctrl_get_name(u32 id) case V4L2_CID_AUTO_FOCUS_STOP: return "Auto Focus, Stop"; case V4L2_CID_AUTO_FOCUS_STATUS:return "Auto Focus, Status"; case V4L2_CID_AUTO_FOCUS_RANGE: return "Auto Focus, Range"; + case V4L2_CID_PAN_SPEED:return "Pan, Speed"; + case V4L2_CID_TILT_SPEED: return "Tilt, Speed"; /* FM Radio Modulator control */ /* Keep the order of the 'case's the same as in videodev2.h! */ diff --git a/include/uapi/linux/v4l2-controls.h b/include/uapi/linux/v4l2-controls.h index 2ac5597..5576044 100644 --- a/include/uapi/linux/v4l2-controls.h +++ b/include/uapi/linux/v4l2-controls.h @@ -745,6 +745,8 @@ enum v4l2_auto_focus_range { V4L2_AUTO_FOCUS_RANGE_INFINITY = 3, }; +#define V4L2_CID_PAN_SPEED (V4L2_CID_CAMERA_CLASS_BASE+32) +#define V4L2_CID_TILT_SPEED(V4L2_CID_CAMERA_CLASS_BASE+33) /* FM Modulator class control IDs */ -- 2.0.0.526.g5318336 -- 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] V4L: uvcvideo: Add support for relative pan/tilt controls
Map V4L2_CID_TILT_RELATIVE and V4L2_CID_PAN_RELATIVE to the standard UVC CT_PANTILT_ABSOLUTE_CONTROL terminal control request. Tested by plugging a Logitech ConferenceCam C3000e USB camera and controlling pan/tilt from the userspace using the VIDIOC_S_CTRL ioctl. Verified that it can pan and tilt at the same time in both directions. Signed-off-by: Vincent Palatin Change-Id: I7b70b228e5c0126683f5f0be34ffd2807f5783dc --- drivers/media/usb/uvc/uvc_ctrl.c | 58 +--- 1 file changed, 55 insertions(+), 3 deletions(-) diff --git a/drivers/media/usb/uvc/uvc_ctrl.c b/drivers/media/usb/uvc/uvc_ctrl.c index 0eb82106..af18120 100644 --- a/drivers/media/usb/uvc/uvc_ctrl.c +++ b/drivers/media/usb/uvc/uvc_ctrl.c @@ -309,9 +309,8 @@ static struct uvc_control_info uvc_ctrls[] = { .selector = UVC_CT_PANTILT_RELATIVE_CONTROL, .index = 12, .size = 4, - .flags = UVC_CTRL_FLAG_SET_CUR | UVC_CTRL_FLAG_GET_MIN - | UVC_CTRL_FLAG_GET_MAX | UVC_CTRL_FLAG_GET_RES - | UVC_CTRL_FLAG_GET_DEF + .flags = UVC_CTRL_FLAG_SET_CUR + | UVC_CTRL_FLAG_GET_RANGE | UVC_CTRL_FLAG_AUTO_UPDATE, }, { @@ -391,6 +390,35 @@ static void uvc_ctrl_set_zoom(struct uvc_control_mapping *mapping, data[2] = min((int)abs(value), 0xff); } +static __s32 uvc_ctrl_get_rel_speed(struct uvc_control_mapping *mapping, + __u8 query, const __u8 *data) +{ + int first = mapping->offset / 8; + __s8 rel = (__s8)data[first]; + + switch (query) { + case UVC_GET_CUR: + return (rel == 0) ? 0 : (rel > 0 ? data[first+1] +: -data[first+1]); + case UVC_GET_MIN: + return -data[first+1]; + case UVC_GET_MAX: + case UVC_GET_RES: + case UVC_GET_DEF: + default: + return data[first+1]; + } +} + +static void uvc_ctrl_set_rel_speed(struct uvc_control_mapping *mapping, + __s32 value, __u8 *data) +{ + int first = mapping->offset / 8; + + data[first] = value == 0 ? 0 : (value > 0) ? 1 : 0xff; + data[first+1] = min_t(int, abs(value), 0xff); +} + static struct uvc_control_mapping uvc_ctrl_mappings[] = { { .id = V4L2_CID_BRIGHTNESS, @@ -677,6 +705,30 @@ static struct uvc_control_mapping uvc_ctrl_mappings[] = { .data_type = UVC_CTRL_DATA_TYPE_SIGNED, }, { + .id = V4L2_CID_PAN_RELATIVE, + .name = "Pan (Relative)", + .entity = UVC_GUID_UVC_CAMERA, + .selector = UVC_CT_PANTILT_RELATIVE_CONTROL, + .size = 16, + .offset = 0, + .v4l2_type = V4L2_CTRL_TYPE_INTEGER, + .data_type = UVC_CTRL_DATA_TYPE_SIGNED, + .get= uvc_ctrl_get_rel_speed, + .set= uvc_ctrl_set_rel_speed, + }, + { + .id = V4L2_CID_TILT_RELATIVE, + .name = "Tilt (Relative)", + .entity = UVC_GUID_UVC_CAMERA, + .selector = UVC_CT_PANTILT_RELATIVE_CONTROL, + .size = 16, + .offset = 16, + .v4l2_type = V4L2_CTRL_TYPE_INTEGER, + .data_type = UVC_CTRL_DATA_TYPE_SIGNED, + .get= uvc_ctrl_get_rel_speed, + .set= uvc_ctrl_set_rel_speed, + }, + { .id = V4L2_CID_PRIVACY, .name = "Privacy", .entity = UVC_GUID_UVC_CAMERA, -- 2.0.0.526.g5318336 -- 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 v2] V4L: uvcvideo: Add support for relative pan/tilt controls
Map V4L2_CID_TILT_RELATIVE and V4L2_CID_PAN_RELATIVE to the standard UVC CT_PANTILT_RELATIVE_CONTROL terminal control request. Tested by plugging a Logitech ConferenceCam C3000e USB camera and controlling pan/tilt from the userspace using the VIDIOC_S_CTRL ioctl. Verified that it can pan and tilt at the same time in both directions. Signed-off-by: Vincent Palatin Change-Id: I7b70b228e5c0126683f5f0be34ffd2807f5783dc --- Changes v2: fix control request name in description. drivers/media/usb/uvc/uvc_ctrl.c | 58 +--- 1 file changed, 55 insertions(+), 3 deletions(-) diff --git a/drivers/media/usb/uvc/uvc_ctrl.c b/drivers/media/usb/uvc/uvc_ctrl.c index 0eb82106..af18120 100644 --- a/drivers/media/usb/uvc/uvc_ctrl.c +++ b/drivers/media/usb/uvc/uvc_ctrl.c @@ -309,9 +309,8 @@ static struct uvc_control_info uvc_ctrls[] = { .selector = UVC_CT_PANTILT_RELATIVE_CONTROL, .index = 12, .size = 4, - .flags = UVC_CTRL_FLAG_SET_CUR | UVC_CTRL_FLAG_GET_MIN - | UVC_CTRL_FLAG_GET_MAX | UVC_CTRL_FLAG_GET_RES - | UVC_CTRL_FLAG_GET_DEF + .flags = UVC_CTRL_FLAG_SET_CUR + | UVC_CTRL_FLAG_GET_RANGE | UVC_CTRL_FLAG_AUTO_UPDATE, }, { @@ -391,6 +390,35 @@ static void uvc_ctrl_set_zoom(struct uvc_control_mapping *mapping, data[2] = min((int)abs(value), 0xff); } +static __s32 uvc_ctrl_get_rel_speed(struct uvc_control_mapping *mapping, + __u8 query, const __u8 *data) +{ + int first = mapping->offset / 8; + __s8 rel = (__s8)data[first]; + + switch (query) { + case UVC_GET_CUR: + return (rel == 0) ? 0 : (rel > 0 ? data[first+1] +: -data[first+1]); + case UVC_GET_MIN: + return -data[first+1]; + case UVC_GET_MAX: + case UVC_GET_RES: + case UVC_GET_DEF: + default: + return data[first+1]; + } +} + +static void uvc_ctrl_set_rel_speed(struct uvc_control_mapping *mapping, + __s32 value, __u8 *data) +{ + int first = mapping->offset / 8; + + data[first] = value == 0 ? 0 : (value > 0) ? 1 : 0xff; + data[first+1] = min_t(int, abs(value), 0xff); +} + static struct uvc_control_mapping uvc_ctrl_mappings[] = { { .id = V4L2_CID_BRIGHTNESS, @@ -677,6 +705,30 @@ static struct uvc_control_mapping uvc_ctrl_mappings[] = { .data_type = UVC_CTRL_DATA_TYPE_SIGNED, }, { + .id = V4L2_CID_PAN_RELATIVE, + .name = "Pan (Relative)", + .entity = UVC_GUID_UVC_CAMERA, + .selector = UVC_CT_PANTILT_RELATIVE_CONTROL, + .size = 16, + .offset = 0, + .v4l2_type = V4L2_CTRL_TYPE_INTEGER, + .data_type = UVC_CTRL_DATA_TYPE_SIGNED, + .get= uvc_ctrl_get_rel_speed, + .set= uvc_ctrl_set_rel_speed, + }, + { + .id = V4L2_CID_TILT_RELATIVE, + .name = "Tilt (Relative)", + .entity = UVC_GUID_UVC_CAMERA, + .selector = UVC_CT_PANTILT_RELATIVE_CONTROL, + .size = 16, + .offset = 16, + .v4l2_type = V4L2_CTRL_TYPE_INTEGER, + .data_type = UVC_CTRL_DATA_TYPE_SIGNED, + .get= uvc_ctrl_get_rel_speed, + .set= uvc_ctrl_set_rel_speed, + }, + { .id = V4L2_CID_PRIVACY, .name = "Privacy", .entity = UVC_GUID_UVC_CAMERA, -- 2.0.0.526.g5318336 -- 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: [RFC 4/4] ARM: dts: exynos5250: Add Spring device tree
Re-sending ... the text-only encoding was not properly turned on on the previous one and irritated the mailing lists. On Mon, Jun 23, 2014 at 9:05 PM, Doug Anderson wrote: > > Andreas, > > On Mon, Jun 23, 2014 at 3:46 PM, Andreas Färber wrote: > > Hi Doug, > > > > Am 23.06.2014 21:47, schrieb Doug Anderson: > >> Thanks for posting! A first pass on this is below... > > > > Thanks a lot for your quick review! My first big .dts patch, and no > > datasheets for the hardware at hand as a user. > > > > A first pass of replies to my defense. ;) > > > >> On Sun, Jun 22, 2014 at 6:21 PM, Andreas Färber wrote: > > [...] > >>> diff --git a/arch/arm/boot/dts/exynos5250-spring.dts > >>> b/arch/arm/boot/dts/exynos5250-spring.dts > >>> new file mode 100644 > >>> index 000..e857d44 > >>> --- /dev/null > >>> +++ b/arch/arm/boot/dts/exynos5250-spring.dts > >>> @@ -0,0 +1,556 @@ > >>> +/* > >>> + * Google Spring board device tree source > >>> + * > >>> + * Copyright (c) 2013 Google, Inc > >>> + * Copyright (c) 2014 SUSE LINUX Products GmbH > >>> + * > >>> + * This program is free software; you can redistribute it and/or modify > >>> + * it under the terms of the GNU General Public License version 2 as > >>> + * published by the Free Software Foundation. > >>> + */ > >>> + > >>> +/dts-v1/; > >>> +#include "exynos5250.dtsi" > >>> +#include "exynos5250-cros-common.dtsi" > >> > >> It is possible we may want to backpedal on the use of > >> "exynos5250-cros-common.dtsi". I know that Olof (now CCed) said he > >> wasn't a fan of how it turned out. > >> > >> The original idea was that it should include the arbitrary set of > >> things that are common between a chunk of Chrome OS boards. As more > >> boards were introduced things would need to migrate from the "common" > >> file to the board files. > >> > >> At the moment the current conventional wisdom is that some duplication > >> is better than the confusing movement of everything back and forth. > >> See exynos5420-peach-pit and exynos5800-peach-pi in ToT linux-next. > >> > >> > >>> +/ { > >>> + model = "Google Spring"; > >>> + compatible = "google,spring", "samsung,exynos5250", > >>> "samsung,exynos5"; > >>> + > >>> + pinctrl@1140 { > >> > >> The new best way to do things is to put this down at the bottom. See > >> exynos5420-peach-pit as an example: > >> > >> &pinctrl_0 { > >> ... > >> } > >> > >> Note that I believe it was decided that top-level references like that > >> should be sorted alphabetically. > > > > Thanks for the hint. (My chosen sort order here was by address.) > > > >> If you wanted to apply that run to exynos5250-snow I don't think it > >> would be a terrible idea. > > > > I can of course apply changes to Snow, but I cannot test them myself. > > If you want to send up a patch like that I'm happy to give it a once > over and also to test it. ...but don't feel obligated > > > >>> + s5m8767_dvs: s5m8767-dvs { > >>> + samsung,pins = "gpd1-0", "gpd1-1", "gpd1-2"; > >>> + samsung,pin-function = <0>; > >>> + samsung,pin-pud = <1>; > >>> + samsung,pin-drv = <0>; > >>> + }; > >>> + > >>> + s5m8767_ds: s5m8767-ds { > >>> + samsung,pins = "gpx2-3", "gpx2-4", "gpx2-5"; > >>> + samsung,pin-function = <0>; > >>> + samsung,pin-pud = <1>; > >>> + samsung,pin-drv = <0>; > >>> + }; > >>> + > >>> + tps65090_irq: tps65090-irq { > >>> + samsung,pins = "gpx2-6"; > >>> + samsung,pin-function = <0>; > >>> + samsung,pin-pud = <0>; > >>> + samsung,pin-drv = <0>; > >>> + }; > >>> + > >>> + s5m8767_irq: s5m8767-irq { > >>> + samsung,pins = "gpx3-2"; > >>> + samsung,pin-function = <0>; > >>> + samsung,pin-pud = <0>; > >>> + samsung,pin-drv = <0>; > >>> + }; > >>> + > >>> + hdmi_hpd_irq: hdmi-hpd-irq { > >>> + samsung,pins = "gpx3-7"; > >>> + samsung,pin-function = <0>; > >>> + samsung,pin-pud = <1>; > >>> + samsung,pin-drv = <0>; > >>> + }; > >>> + }; > >>> + > >>> + pinctrl@1340 { > >>> + hsic_reset: hsic-reset { > >>> + samsung,pins = "gpe1-0"; > >>> + samsung,pin-function = <1>; > >>> + samsung,pin-pud = <0>; > >>> + samsung,pin-drv = <0>; > >>> + }; > >> > >> I'm pretty sure that the HSIC reset needed some funky code to make it > >> work and I'm not sure what the status of that is upstream > > > > Yeah, you mentioned something along those lines. How
Re: [PATCH v2 1/2] [media] V4L: Add camera pan/tilt speed controls
On Tue, Sep 2, 2014 at 9:54 PM, Pawel Osciak wrote: > On Sat, Aug 16, 2014 at 4:08 AM, Vincent Palatin > wrote: >> >> The V4L2_CID_PAN_SPEED and V4L2_CID_TILT_SPEED controls allow to move the >> camera by setting its rotation speed around its axis. >> >> Signed-off-by: Vincent Palatin > > Reviewed-by: Pawel Osciak > >> >> --- >> Changes from v1: >> - update the documentation wording according to Pawel suggestion. >> >> Documentation/DocBook/media/v4l/compat.xml | 10 ++ >> Documentation/DocBook/media/v4l/controls.xml | 21 + >> drivers/media/v4l2-core/v4l2-ctrls.c | 2 ++ >> include/uapi/linux/v4l2-controls.h | 2 ++ >> 4 files changed, 35 insertions(+) >> >> diff --git a/Documentation/DocBook/media/v4l/compat.xml >> b/Documentation/DocBook/media/v4l/compat.xml >> index eee6f0f..21910e9 100644 >> --- a/Documentation/DocBook/media/v4l/compat.xml >> +++ b/Documentation/DocBook/media/v4l/compat.xml >> @@ -2545,6 +2545,16 @@ fields changed from _s32 to _u32. >> >> >> >> + >> + V4L2 in Linux 3.17 > > This will need a bump. Yes, I did not expect to miss 3.17 window. I will send an updated v3 patch. >> >> + >> + >> + Added V4L2_CID_PAN_SPEED and >> + V4L2_CID_TILT_SPEED camera controls. >> + >> + >> + >> + >> >>Relation of V4L2 to other Linux multimedia APIs >> >> diff --git a/Documentation/DocBook/media/v4l/controls.xml >> b/Documentation/DocBook/media/v4l/controls.xml >> index 47198ee..be88e64 100644 >> --- a/Documentation/DocBook/media/v4l/controls.xml >> +++ b/Documentation/DocBook/media/v4l/controls.xml >> @@ -3914,6 +3914,27 @@ by exposure, white balance or focus controls. >> >> >> >> + >> + > spanname="id">V4L2_CID_PAN_SPEED >> + integer >> + This control turns the >> +camera horizontally at the specific speed. The unit is undefined. A >> +positive value moves the camera to the right (clockwise when viewed >> +from above), a negative value to the left. A value of zero stops the motion >> +if one is in progress and has no effect otherwise. >> + >> + >> + >> + >> + > spanname="id">V4L2_CID_TILT_SPEED >> + integer >> + This control turns the >> +camera vertically at the specified speed. The unit is undefined. A >> +positive value moves the camera up, a negative value down. A value of zero >> +stops the motion if one is in progress and has no effect otherwise. >> + >> + >> + >> >> >> >> diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c >> b/drivers/media/v4l2-core/v4l2-ctrls.c >> index 55c6832..57ddaf4 100644 >> --- a/drivers/media/v4l2-core/v4l2-ctrls.c >> +++ b/drivers/media/v4l2-core/v4l2-ctrls.c >> @@ -787,6 +787,8 @@ const char *v4l2_ctrl_get_name(u32 id) >> case V4L2_CID_AUTO_FOCUS_STOP: return "Auto Focus, Stop"; >> case V4L2_CID_AUTO_FOCUS_STATUS:return "Auto Focus, Status"; >> case V4L2_CID_AUTO_FOCUS_RANGE: return "Auto Focus, Range"; >> + case V4L2_CID_PAN_SPEED:return "Pan, Speed"; >> + case V4L2_CID_TILT_SPEED: return "Tilt, Speed"; >> >> /* FM Radio Modulator control */ >> /* Keep the order of the 'case's the same as in videodev2.h! */ >> diff --git a/include/uapi/linux/v4l2-controls.h >> b/include/uapi/linux/v4l2-controls.h >> index 2ac5597..5576044 100644 >> --- a/include/uapi/linux/v4l2-controls.h >> +++ b/include/uapi/linux/v4l2-controls.h >> @@ -745,6 +745,8 @@ enum v4l2_auto_focus_range { >> V4L2_AUTO_FOCUS_RANGE_INFINITY = 3, >> }; >> >> +#define V4L2_CID_PAN_SPEED >> (V4L2_CID_CAMERA_CLASS_BASE+32) >> +#define V4L2_CID_TILT_SPEED >> (V4L2_CID_CAMERA_CLASS_BASE+33) >> >> /* FM Modulator class control IDs */ >> >> -- >> 2.1.0.rc2.206.gedb03e5 >> -- 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 1/2] [media] V4L: Add camera pan/tilt speed controls
The V4L2_CID_PAN_SPEED and V4L2_CID_TILT_SPEED controls allow to move the camera by setting its rotation speed around its axis. Signed-off-by: Vincent Palatin --- Changes from v1: - update the documentation wording according to Pawel suggestion. Changes from v2: - bump Linux kernel version for the API change. Documentation/DocBook/media/v4l/compat.xml | 10 ++ Documentation/DocBook/media/v4l/controls.xml | 21 + drivers/media/v4l2-core/v4l2-ctrls.c | 2 ++ include/uapi/linux/v4l2-controls.h | 2 ++ 4 files changed, 35 insertions(+) diff --git a/Documentation/DocBook/media/v4l/compat.xml b/Documentation/DocBook/media/v4l/compat.xml index eee6f0f..7aa7c5d 100644 --- a/Documentation/DocBook/media/v4l/compat.xml +++ b/Documentation/DocBook/media/v4l/compat.xml @@ -2545,6 +2545,16 @@ fields changed from _s32 to _u32. + + V4L2 in Linux 3.18 + + + Added V4L2_CID_PAN_SPEED and + V4L2_CID_TILT_SPEED camera controls. + + + + Relation of V4L2 to other Linux multimedia APIs diff --git a/Documentation/DocBook/media/v4l/controls.xml b/Documentation/DocBook/media/v4l/controls.xml index 9f5ffd8..124f287 100644 --- a/Documentation/DocBook/media/v4l/controls.xml +++ b/Documentation/DocBook/media/v4l/controls.xml @@ -3965,6 +3965,27 @@ by exposure, white balance or focus controls. + + V4L2_CID_PAN_SPEED + integer + This control turns the +camera horizontally at the specific speed. The unit is undefined. A +positive value moves the camera to the right (clockwise when viewed +from above), a negative value to the left. A value of zero stops the motion +if one is in progress and has no effect otherwise. + + + + + V4L2_CID_TILT_SPEED + integer + This control turns the +camera vertically at the specified speed. The unit is undefined. A +positive value moves the camera up, a negative value down. A value of zero +stops the motion if one is in progress and has no effect otherwise. + + + diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c b/drivers/media/v4l2-core/v4l2-ctrls.c index f030d6a..4d050f9 100644 --- a/drivers/media/v4l2-core/v4l2-ctrls.c +++ b/drivers/media/v4l2-core/v4l2-ctrls.c @@ -796,6 +796,8 @@ const char *v4l2_ctrl_get_name(u32 id) case V4L2_CID_AUTO_FOCUS_STOP: return "Auto Focus, Stop"; case V4L2_CID_AUTO_FOCUS_STATUS:return "Auto Focus, Status"; case V4L2_CID_AUTO_FOCUS_RANGE: return "Auto Focus, Range"; + case V4L2_CID_PAN_SPEED:return "Pan, Speed"; + case V4L2_CID_TILT_SPEED: return "Tilt, Speed"; /* FM Radio Modulator controls */ /* Keep the order of the 'case's the same as in v4l2-controls.h! */ diff --git a/include/uapi/linux/v4l2-controls.h b/include/uapi/linux/v4l2-controls.h index e946e43..4de238b 100644 --- a/include/uapi/linux/v4l2-controls.h +++ b/include/uapi/linux/v4l2-controls.h @@ -746,6 +746,8 @@ enum v4l2_auto_focus_range { V4L2_AUTO_FOCUS_RANGE_INFINITY = 3, }; +#define V4L2_CID_PAN_SPEED (V4L2_CID_CAMERA_CLASS_BASE+32) +#define V4L2_CID_TILT_SPEED(V4L2_CID_CAMERA_CLASS_BASE+33) /* FM Modulator class control IDs */ -- 2.1.0.rc2.206.gedb03e5 -- 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 2/2] V4L: uvcvideo: Add support for pan/tilt speed controls
Map V4L2_CID_TILT_SPEED and V4L2_CID_PAN_SPEED to the standard UVC CT_PANTILT_RELATIVE_CONTROL terminal control request. Tested by plugging a Logitech ConferenceCam C3000e USB camera and controlling pan/tilt from the userspace using the VIDIOC_S_CTRL ioctl. Verified that it can pan and tilt at the same time in both directions. Signed-off-by: Vincent Palatin Change-Id: I7b70b228e5c0126683f5f0be34ffd2807f5783dc --- Changes from v1/v2: - rebased drivers/media/usb/uvc/uvc_ctrl.c | 58 +--- 1 file changed, 55 insertions(+), 3 deletions(-) diff --git a/drivers/media/usb/uvc/uvc_ctrl.c b/drivers/media/usb/uvc/uvc_ctrl.c index 0eb82106..d703cb0 100644 --- a/drivers/media/usb/uvc/uvc_ctrl.c +++ b/drivers/media/usb/uvc/uvc_ctrl.c @@ -309,9 +309,8 @@ static struct uvc_control_info uvc_ctrls[] = { .selector = UVC_CT_PANTILT_RELATIVE_CONTROL, .index = 12, .size = 4, - .flags = UVC_CTRL_FLAG_SET_CUR | UVC_CTRL_FLAG_GET_MIN - | UVC_CTRL_FLAG_GET_MAX | UVC_CTRL_FLAG_GET_RES - | UVC_CTRL_FLAG_GET_DEF + .flags = UVC_CTRL_FLAG_SET_CUR + | UVC_CTRL_FLAG_GET_RANGE | UVC_CTRL_FLAG_AUTO_UPDATE, }, { @@ -391,6 +390,35 @@ static void uvc_ctrl_set_zoom(struct uvc_control_mapping *mapping, data[2] = min((int)abs(value), 0xff); } +static __s32 uvc_ctrl_get_rel_speed(struct uvc_control_mapping *mapping, + __u8 query, const __u8 *data) +{ + int first = mapping->offset / 8; + __s8 rel = (__s8)data[first]; + + switch (query) { + case UVC_GET_CUR: + return (rel == 0) ? 0 : (rel > 0 ? data[first+1] +: -data[first+1]); + case UVC_GET_MIN: + return -data[first+1]; + case UVC_GET_MAX: + case UVC_GET_RES: + case UVC_GET_DEF: + default: + return data[first+1]; + } +} + +static void uvc_ctrl_set_rel_speed(struct uvc_control_mapping *mapping, + __s32 value, __u8 *data) +{ + int first = mapping->offset / 8; + + data[first] = value == 0 ? 0 : (value > 0) ? 1 : 0xff; + data[first+1] = min_t(int, abs(value), 0xff); +} + static struct uvc_control_mapping uvc_ctrl_mappings[] = { { .id = V4L2_CID_BRIGHTNESS, @@ -677,6 +705,30 @@ static struct uvc_control_mapping uvc_ctrl_mappings[] = { .data_type = UVC_CTRL_DATA_TYPE_SIGNED, }, { + .id = V4L2_CID_PAN_SPEED, + .name = "Pan (Speed)", + .entity = UVC_GUID_UVC_CAMERA, + .selector = UVC_CT_PANTILT_RELATIVE_CONTROL, + .size = 16, + .offset = 0, + .v4l2_type = V4L2_CTRL_TYPE_INTEGER, + .data_type = UVC_CTRL_DATA_TYPE_SIGNED, + .get= uvc_ctrl_get_rel_speed, + .set= uvc_ctrl_set_rel_speed, + }, + { + .id = V4L2_CID_TILT_SPEED, + .name = "Tilt (Speed)", + .entity = UVC_GUID_UVC_CAMERA, + .selector = UVC_CT_PANTILT_RELATIVE_CONTROL, + .size = 16, + .offset = 16, + .v4l2_type = V4L2_CTRL_TYPE_INTEGER, + .data_type = UVC_CTRL_DATA_TYPE_SIGNED, + .get= uvc_ctrl_get_rel_speed, + .set= uvc_ctrl_set_rel_speed, + }, + { .id = V4L2_CID_PRIVACY, .name = "Privacy", .entity = UVC_GUID_UVC_CAMERA, -- 2.1.0.rc2.206.gedb03e5 -- 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 2/2] V4L: uvcvideo: Add support for pan/tilt speed controls
Map V4L2_CID_TILT_SPEED and V4L2_CID_PAN_SPEED to the standard UVC CT_PANTILT_RELATIVE_CONTROL terminal control request. Tested by plugging a Logitech ConferenceCam C3000e USB camera and controlling pan/tilt from the userspace using the VIDIOC_S_CTRL ioctl. Verified that it can pan and tilt at the same time in both directions. Signed-off-by: Vincent Palatin --- Changes from v1/v2: - rebased Changes from v3: - removed gerrit-id drivers/media/usb/uvc/uvc_ctrl.c | 58 +--- 1 file changed, 55 insertions(+), 3 deletions(-) diff --git a/drivers/media/usb/uvc/uvc_ctrl.c b/drivers/media/usb/uvc/uvc_ctrl.c index 0eb82106..d703cb0 100644 --- a/drivers/media/usb/uvc/uvc_ctrl.c +++ b/drivers/media/usb/uvc/uvc_ctrl.c @@ -309,9 +309,8 @@ static struct uvc_control_info uvc_ctrls[] = { .selector = UVC_CT_PANTILT_RELATIVE_CONTROL, .index = 12, .size = 4, - .flags = UVC_CTRL_FLAG_SET_CUR | UVC_CTRL_FLAG_GET_MIN - | UVC_CTRL_FLAG_GET_MAX | UVC_CTRL_FLAG_GET_RES - | UVC_CTRL_FLAG_GET_DEF + .flags = UVC_CTRL_FLAG_SET_CUR + | UVC_CTRL_FLAG_GET_RANGE | UVC_CTRL_FLAG_AUTO_UPDATE, }, { @@ -391,6 +390,35 @@ static void uvc_ctrl_set_zoom(struct uvc_control_mapping *mapping, data[2] = min((int)abs(value), 0xff); } +static __s32 uvc_ctrl_get_rel_speed(struct uvc_control_mapping *mapping, + __u8 query, const __u8 *data) +{ + int first = mapping->offset / 8; + __s8 rel = (__s8)data[first]; + + switch (query) { + case UVC_GET_CUR: + return (rel == 0) ? 0 : (rel > 0 ? data[first+1] +: -data[first+1]); + case UVC_GET_MIN: + return -data[first+1]; + case UVC_GET_MAX: + case UVC_GET_RES: + case UVC_GET_DEF: + default: + return data[first+1]; + } +} + +static void uvc_ctrl_set_rel_speed(struct uvc_control_mapping *mapping, + __s32 value, __u8 *data) +{ + int first = mapping->offset / 8; + + data[first] = value == 0 ? 0 : (value > 0) ? 1 : 0xff; + data[first+1] = min_t(int, abs(value), 0xff); +} + static struct uvc_control_mapping uvc_ctrl_mappings[] = { { .id = V4L2_CID_BRIGHTNESS, @@ -677,6 +705,30 @@ static struct uvc_control_mapping uvc_ctrl_mappings[] = { .data_type = UVC_CTRL_DATA_TYPE_SIGNED, }, { + .id = V4L2_CID_PAN_SPEED, + .name = "Pan (Speed)", + .entity = UVC_GUID_UVC_CAMERA, + .selector = UVC_CT_PANTILT_RELATIVE_CONTROL, + .size = 16, + .offset = 0, + .v4l2_type = V4L2_CTRL_TYPE_INTEGER, + .data_type = UVC_CTRL_DATA_TYPE_SIGNED, + .get= uvc_ctrl_get_rel_speed, + .set= uvc_ctrl_set_rel_speed, + }, + { + .id = V4L2_CID_TILT_SPEED, + .name = "Tilt (Speed)", + .entity = UVC_GUID_UVC_CAMERA, + .selector = UVC_CT_PANTILT_RELATIVE_CONTROL, + .size = 16, + .offset = 16, + .v4l2_type = V4L2_CTRL_TYPE_INTEGER, + .data_type = UVC_CTRL_DATA_TYPE_SIGNED, + .get= uvc_ctrl_get_rel_speed, + .set= uvc_ctrl_set_rel_speed, + }, + { .id = V4L2_CID_PRIVACY, .name = "Privacy", .entity = UVC_GUID_UVC_CAMERA, -- 2.1.0.rc2.206.gedb03e5 -- 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/2] [media] V4L: Add camera pan/tilt speed controls
On Sun, Aug 3, 2014 at 10:52 PM, Pawel Osciak wrote: > This looks good to me in general (with one comment below). I don't think we > can easily implement current V4L2 pan and tilt controls that are for > movement by a specified amount in terms of UVC pan/tilt speed controls, > which only let us set speed and direction... > > On Wed, Jul 9, 2014 at 8:49 AM, Vincent Palatin > wrote: >> >> The V4L2_CID_PAN_SPEED and V4L2_CID_TILT_SPEED controls allow to move the >> camera by setting its rotation speed around its axis. >> >> Signed-off-by: Vincent Palatin >> >> --- >> Documentation/DocBook/media/v4l/compat.xml | 10 ++ >> Documentation/DocBook/media/v4l/controls.xml | 21 + >> drivers/media/v4l2-core/v4l2-ctrls.c | 2 ++ >> include/uapi/linux/v4l2-controls.h | 2 ++ >> 4 files changed, 35 insertions(+) >> >> diff --git a/Documentation/DocBook/media/v4l/compat.xml >> b/Documentation/DocBook/media/v4l/compat.xml >> index eee6f0f..21910e9 100644 >> --- a/Documentation/DocBook/media/v4l/compat.xml >> +++ b/Documentation/DocBook/media/v4l/compat.xml >> @@ -2545,6 +2545,16 @@ fields changed from _s32 to _u32. >> >> >> >> + >> + V4L2 in Linux 3.17 >> + >> + >> + Added V4L2_CID_PAN_SPEED and >> + V4L2_CID_TILT_SPEED camera controls. >> + >> + >> + >> + >> >>Relation of V4L2 to other Linux multimedia APIs >> >> diff --git a/Documentation/DocBook/media/v4l/controls.xml >> b/Documentation/DocBook/media/v4l/controls.xml >> index 47198ee..cdf97f0 100644 >> --- a/Documentation/DocBook/media/v4l/controls.xml >> +++ b/Documentation/DocBook/media/v4l/controls.xml >> @@ -3914,6 +3914,27 @@ by exposure, white balance or focus >> controls. >> >> >> >> + >> + > spanname="id">V4L2_CID_PAN_SPEED >> + integer >> + This control turns the >> +camera horizontally at the specific speed. The unit is undefined. A >> +positive value moves the camera to the right (clockwise when viewed >> +from above), a negative value to the left. A value of zero does not >> +cause or stop the motion. > > > How do we stop/start? As mentioned in the last sentence of the paragraph above, setting 0 stops the movement. setting non-zero value starts it if needed. > >> >> + >> + >> + >> + >> + > spanname="id">V4L2_CID_TILT_SPEED >> + integer >> + This control turns the >> +camera vertically at the specified speed. The unit is undefined. A >> +positive value moves the camera up, a negative value down. A value of >> +zero does not cause or stop the motion. >> + >> + >> + >> >> >> >> diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c >> b/drivers/media/v4l2-core/v4l2-ctrls.c >> index 55c6832..57ddaf4 100644 >> --- a/drivers/media/v4l2-core/v4l2-ctrls.c >> +++ b/drivers/media/v4l2-core/v4l2-ctrls.c >> @@ -787,6 +787,8 @@ const char *v4l2_ctrl_get_name(u32 id) >> case V4L2_CID_AUTO_FOCUS_STOP: return "Auto Focus, Stop"; >> case V4L2_CID_AUTO_FOCUS_STATUS:return "Auto Focus, >> Status"; >> case V4L2_CID_AUTO_FOCUS_RANGE: return "Auto Focus, >> Range"; >> + case V4L2_CID_PAN_SPEED:return "Pan, Speed"; >> + case V4L2_CID_TILT_SPEED: return "Tilt, Speed"; >> >> /* FM Radio Modulator control */ >> /* Keep the order of the 'case's the same as in videodev2.h! */ >> diff --git a/include/uapi/linux/v4l2-controls.h >> b/include/uapi/linux/v4l2-controls.h >> index 2ac5597..5576044 100644 >> --- a/include/uapi/linux/v4l2-controls.h >> +++ b/include/uapi/linux/v4l2-controls.h >> @@ -745,6 +745,8 @@ enum v4l2_auto_focus_range { >> V4L2_AUTO_FOCUS_RANGE_INFINITY = 3, >> }; >> >> +#define V4L2_CID_PAN_SPEED >> (V4L2_CID_CAMERA_CLASS_BASE+32) >> +#define V4L2_CID_TILT_SPEED >> (V4L2_CID_CAMERA_CLASS_BASE+33) >> >> /* FM Modulator class control IDs */ >> >> -- >> 2.0.0.526.g5318336 >> > -- 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/2] [media] V4L: Add camera pan/tilt speed controls
On Wed, Aug 6, 2014 at 7:18 PM, Pawel Osciak wrote: > > On Thu, Aug 7, 2014 at 12:10 AM, Vincent Palatin > wrote: > > > > On Sun, Aug 3, 2014 at 10:52 PM, Pawel Osciak wrote: > > > This looks good to me in general (with one comment below). I don't think > > > we > > > can easily implement current V4L2 pan and tilt controls that are for > > > movement by a specified amount in terms of UVC pan/tilt speed controls, > > > which only let us set speed and direction... > > > > > > On Wed, Jul 9, 2014 at 8:49 AM, Vincent Palatin > > > wrote: > > >> > > >> The V4L2_CID_PAN_SPEED and V4L2_CID_TILT_SPEED controls allow to move the > > >> camera by setting its rotation speed around its axis. > > >> > > >> Signed-off-by: Vincent Palatin > > >> > > >> --- > > >> Documentation/DocBook/media/v4l/compat.xml | 10 ++ > > >> Documentation/DocBook/media/v4l/controls.xml | 21 + > > >> drivers/media/v4l2-core/v4l2-ctrls.c | 2 ++ > > >> include/uapi/linux/v4l2-controls.h | 2 ++ > > >> 4 files changed, 35 insertions(+) > > >> > > >> diff --git a/Documentation/DocBook/media/v4l/compat.xml > > >> b/Documentation/DocBook/media/v4l/compat.xml > > >> index eee6f0f..21910e9 100644 > > >> --- a/Documentation/DocBook/media/v4l/compat.xml > > >> +++ b/Documentation/DocBook/media/v4l/compat.xml > > >> @@ -2545,6 +2545,16 @@ fields changed from _s32 to _u32. > > >> > > >> > > >> > > >> + > > >> + V4L2 in Linux 3.17 > > >> + > > >> + > > >> + Added V4L2_CID_PAN_SPEED and > > >> + V4L2_CID_TILT_SPEED camera controls. > > >> + > > >> + > > >> + > > >> + > > >> > > >>Relation of V4L2 to other Linux multimedia APIs > > >> > > >> diff --git a/Documentation/DocBook/media/v4l/controls.xml > > >> b/Documentation/DocBook/media/v4l/controls.xml > > >> index 47198ee..cdf97f0 100644 > > >> --- a/Documentation/DocBook/media/v4l/controls.xml > > >> +++ b/Documentation/DocBook/media/v4l/controls.xml > > >> @@ -3914,6 +3914,27 @@ by exposure, white balance or focus > > >> controls. > > >> > > >> > > >> > > >> + > > >> +> >> spanname="id">V4L2_CID_PAN_SPEED > > >> + integer > > >> + This control turns the > > >> +camera horizontally at the specific speed. The unit is undefined. A > > >> +positive value moves the camera to the right (clockwise when viewed > > >> +from above), a negative value to the left. A value of zero does not > > >> +cause or stop the motion. > > > > > > > > > How do we stop/start? > > > > As mentioned in the last sentence of the paragraph above, setting 0 > > stops the movement. > > setting non-zero value starts it if needed. > > > > The sentence says "A value of zero does *not* cause or stop the > motion.". Perhaps "not" is a typo then? maybe my phrasing is really bad but the "not" isn't a typo. The developed version would be : "A value of zero does *not* cause [any motion if the camera is already stopped] or stop the motion [if it is currently moving with a non-zero speed]" -- Vincent -- 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 v2 1/2] [media] V4L: Add camera pan/tilt speed controls
The V4L2_CID_PAN_SPEED and V4L2_CID_TILT_SPEED controls allow to move the camera by setting its rotation speed around its axis. Signed-off-by: Vincent Palatin --- Changes from v1: - update the documentation wording according to Pawel suggestion. Documentation/DocBook/media/v4l/compat.xml | 10 ++ Documentation/DocBook/media/v4l/controls.xml | 21 + drivers/media/v4l2-core/v4l2-ctrls.c | 2 ++ include/uapi/linux/v4l2-controls.h | 2 ++ 4 files changed, 35 insertions(+) diff --git a/Documentation/DocBook/media/v4l/compat.xml b/Documentation/DocBook/media/v4l/compat.xml index eee6f0f..21910e9 100644 --- a/Documentation/DocBook/media/v4l/compat.xml +++ b/Documentation/DocBook/media/v4l/compat.xml @@ -2545,6 +2545,16 @@ fields changed from _s32 to _u32. + + V4L2 in Linux 3.17 + + + Added V4L2_CID_PAN_SPEED and + V4L2_CID_TILT_SPEED camera controls. + + + + Relation of V4L2 to other Linux multimedia APIs diff --git a/Documentation/DocBook/media/v4l/controls.xml b/Documentation/DocBook/media/v4l/controls.xml index 47198ee..be88e64 100644 --- a/Documentation/DocBook/media/v4l/controls.xml +++ b/Documentation/DocBook/media/v4l/controls.xml @@ -3914,6 +3914,27 @@ by exposure, white balance or focus controls. + + V4L2_CID_PAN_SPEED + integer + This control turns the +camera horizontally at the specific speed. The unit is undefined. A +positive value moves the camera to the right (clockwise when viewed +from above), a negative value to the left. A value of zero stops the motion +if one is in progress and has no effect otherwise. + + + + + V4L2_CID_TILT_SPEED + integer + This control turns the +camera vertically at the specified speed. The unit is undefined. A +positive value moves the camera up, a negative value down. A value of zero +stops the motion if one is in progress and has no effect otherwise. + + + diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c b/drivers/media/v4l2-core/v4l2-ctrls.c index 55c6832..57ddaf4 100644 --- a/drivers/media/v4l2-core/v4l2-ctrls.c +++ b/drivers/media/v4l2-core/v4l2-ctrls.c @@ -787,6 +787,8 @@ const char *v4l2_ctrl_get_name(u32 id) case V4L2_CID_AUTO_FOCUS_STOP: return "Auto Focus, Stop"; case V4L2_CID_AUTO_FOCUS_STATUS:return "Auto Focus, Status"; case V4L2_CID_AUTO_FOCUS_RANGE: return "Auto Focus, Range"; + case V4L2_CID_PAN_SPEED:return "Pan, Speed"; + case V4L2_CID_TILT_SPEED: return "Tilt, Speed"; /* FM Radio Modulator control */ /* Keep the order of the 'case's the same as in videodev2.h! */ diff --git a/include/uapi/linux/v4l2-controls.h b/include/uapi/linux/v4l2-controls.h index 2ac5597..5576044 100644 --- a/include/uapi/linux/v4l2-controls.h +++ b/include/uapi/linux/v4l2-controls.h @@ -745,6 +745,8 @@ enum v4l2_auto_focus_range { V4L2_AUTO_FOCUS_RANGE_INFINITY = 3, }; +#define V4L2_CID_PAN_SPEED (V4L2_CID_CAMERA_CLASS_BASE+32) +#define V4L2_CID_TILT_SPEED(V4L2_CID_CAMERA_CLASS_BASE+33) /* FM Modulator class control IDs */ -- 2.1.0.rc2.206.gedb03e5 -- 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 2/2] V4L: uvcvideo: Add support for pan/tilt speed controls
On Thu, Sep 4, 2014 at 1:29 PM, Laurent Pinchart wrote: > > Hi Vincent, > > Thank you for the patch. > > On Wednesday 03 September 2014 17:47:48 Vincent Palatin wrote: > > Map V4L2_CID_TILT_SPEED and V4L2_CID_PAN_SPEED to the standard UVC > > CT_PANTILT_RELATIVE_CONTROL terminal control request. > > > > Tested by plugging a Logitech ConferenceCam C3000e USB camera > > and controlling pan/tilt from the userspace using the VIDIOC_S_CTRL ioctl. > > Verified that it can pan and tilt at the same time in both directions. > > > > Signed-off-by: Vincent Palatin > > Small comment here, as Pawel has reviewed the previous version, you could have > added his Reviewed-by tag to the patch. > > > --- > > Changes from v1/v2: > > - rebased > > Changes from v3: > > - removed gerrit-id > > > > drivers/media/usb/uvc/uvc_ctrl.c | 58 ++--- > > 1 file changed, 55 insertions(+), 3 deletions(-) > > > > diff --git a/drivers/media/usb/uvc/uvc_ctrl.c > > b/drivers/media/usb/uvc/uvc_ctrl.c index 0eb82106..d703cb0 100644 > > --- a/drivers/media/usb/uvc/uvc_ctrl.c > > +++ b/drivers/media/usb/uvc/uvc_ctrl.c > > @@ -309,9 +309,8 @@ static struct uvc_control_info uvc_ctrls[] = { > > .selector = UVC_CT_PANTILT_RELATIVE_CONTROL, > > .index = 12, > > .size = 4, > > - .flags = UVC_CTRL_FLAG_SET_CUR | > > UVC_CTRL_FLAG_GET_MIN > > - | UVC_CTRL_FLAG_GET_MAX | > > UVC_CTRL_FLAG_GET_RES > > - | UVC_CTRL_FLAG_GET_DEF > > + .flags = UVC_CTRL_FLAG_SET_CUR > > + | UVC_CTRL_FLAG_GET_RANGE > > | UVC_CTRL_FLAG_AUTO_UPDATE, > > }, > > { > > @@ -391,6 +390,35 @@ static void uvc_ctrl_set_zoom(struct > > uvc_control_mapping *mapping, data[2] = min((int)abs(value), 0xff); > > } > > > > +static __s32 uvc_ctrl_get_rel_speed(struct uvc_control_mapping *mapping, > > + __u8 query, const __u8 *data) > > +{ > > + int first = mapping->offset / 8; > > Nitpicking, I would use unsigned int instead of int here, as the value can't > be negative (same comment for the next function). > > If you're fine with that there's no need to resubmit, I can modify this when > applying. Yes, I'm fine with that. > > > > + __s8 rel = (__s8)data[first]; > > + > > + switch (query) { > > + case UVC_GET_CUR: > > + return (rel == 0) ? 0 : (rel > 0 ? data[first+1] > > + : -data[first+1]); > > + case UVC_GET_MIN: > > + return -data[first+1]; > > + case UVC_GET_MAX: > > + case UVC_GET_RES: > > + case UVC_GET_DEF: > > + default: > > + return data[first+1]; > > + } > > +} > > + > > +static void uvc_ctrl_set_rel_speed(struct uvc_control_mapping *mapping, > > + __s32 value, __u8 *data) > > +{ > > + int first = mapping->offset / 8; > > + > > + data[first] = value == 0 ? 0 : (value > 0) ? 1 : 0xff; > > + data[first+1] = min_t(int, abs(value), 0xff); > > +} > > + > > static struct uvc_control_mapping uvc_ctrl_mappings[] = { > > { > > .id = V4L2_CID_BRIGHTNESS, > > @@ -677,6 +705,30 @@ static struct uvc_control_mapping uvc_ctrl_mappings[] = > > { .data_type = UVC_CTRL_DATA_TYPE_SIGNED, > > }, > > { > > + .id = V4L2_CID_PAN_SPEED, > > + .name = "Pan (Speed)", > > + .entity = UVC_GUID_UVC_CAMERA, > > + .selector = UVC_CT_PANTILT_RELATIVE_CONTROL, > > + .size = 16, > > + .offset = 0, > > + .v4l2_type = V4L2_CTRL_TYPE_INTEGER, > > + .data_type = UVC_CTRL_DATA_TYPE_SIGNED, > > + .get= uvc_ctrl_get_rel_speed, > > + .set= uvc_ctrl_set_rel_speed, > > + }, > > + { > > + .id = V4L2_CID_TILT_SPEED, > > + .name = "Tilt (Speed)", > > + .entity = UVC_GUID_UVC_CAMERA, > > + .selector = UVC_CT_PANTILT_RELATIVE_CONTROL, > > + .size = 16, > > + .offset = 16, > > +
Re: [PATCH v4 2/2] V4L: uvcvideo: Add support for pan/tilt speed controls
On Thu, Sep 4, 2014 at 1:35 PM, Laurent Pinchart wrote: > > Hi Vincent, > > On Wednesday 03 September 2014 17:47:48 Vincent Palatin wrote: > > Map V4L2_CID_TILT_SPEED and V4L2_CID_PAN_SPEED to the standard UVC > > CT_PANTILT_RELATIVE_CONTROL terminal control request. > > > > Tested by plugging a Logitech ConferenceCam C3000e USB camera > > and controlling pan/tilt from the userspace using the VIDIOC_S_CTRL ioctl. > > Verified that it can pan and tilt at the same time in both directions. > > By the way, what is the control value reported by the device after it stops > moving by itself due to reaching a limit position ? I don't know. I no longer have the device in hand, I will ask somebody who has one to experiment and check. > > > > Signed-off-by: Vincent Palatin > > --- > > Changes from v1/v2: > > - rebased > > Changes from v3: > > - removed gerrit-id > > > > drivers/media/usb/uvc/uvc_ctrl.c | 58 ++--- > > 1 file changed, 55 insertions(+), 3 deletions(-) > > > > diff --git a/drivers/media/usb/uvc/uvc_ctrl.c > > b/drivers/media/usb/uvc/uvc_ctrl.c index 0eb82106..d703cb0 100644 > > --- a/drivers/media/usb/uvc/uvc_ctrl.c > > +++ b/drivers/media/usb/uvc/uvc_ctrl.c > > @@ -309,9 +309,8 @@ static struct uvc_control_info uvc_ctrls[] = { > > .selector = UVC_CT_PANTILT_RELATIVE_CONTROL, > > .index = 12, > > .size = 4, > > - .flags = UVC_CTRL_FLAG_SET_CUR | > > UVC_CTRL_FLAG_GET_MIN > > - | UVC_CTRL_FLAG_GET_MAX | > > UVC_CTRL_FLAG_GET_RES > > - | UVC_CTRL_FLAG_GET_DEF > > + .flags = UVC_CTRL_FLAG_SET_CUR > > + | UVC_CTRL_FLAG_GET_RANGE > > > > | UVC_CTRL_FLAG_AUTO_UPDATE, > > > > }, > > { > > @@ -391,6 +390,35 @@ static void uvc_ctrl_set_zoom(struct > > uvc_control_mapping *mapping, data[2] = min((int)abs(value), 0xff); > > } > > > > +static __s32 uvc_ctrl_get_rel_speed(struct uvc_control_mapping *mapping, > > + __u8 query, const __u8 *data) > > +{ > > + int first = mapping->offset / 8; > > + __s8 rel = (__s8)data[first]; > > + > > + switch (query) { > > + case UVC_GET_CUR: > > + return (rel == 0) ? 0 : (rel > 0 ? data[first+1] > > + : -data[first+1]); > > + case UVC_GET_MIN: > > + return -data[first+1]; > > + case UVC_GET_MAX: > > + case UVC_GET_RES: > > + case UVC_GET_DEF: > > + default: > > + return data[first+1]; > > + } > > +} > > + > > +static void uvc_ctrl_set_rel_speed(struct uvc_control_mapping *mapping, > > + __s32 value, __u8 *data) > > +{ > > + int first = mapping->offset / 8; > > + > > + data[first] = value == 0 ? 0 : (value > 0) ? 1 : 0xff; > > + data[first+1] = min_t(int, abs(value), 0xff); > > +} > > + > > static struct uvc_control_mapping uvc_ctrl_mappings[] = { > > { > > .id = V4L2_CID_BRIGHTNESS, > > @@ -677,6 +705,30 @@ static struct uvc_control_mapping uvc_ctrl_mappings[] = > > { .data_type = UVC_CTRL_DATA_TYPE_SIGNED, > > }, > > { > > + .id = V4L2_CID_PAN_SPEED, > > + .name = "Pan (Speed)", > > + .entity = UVC_GUID_UVC_CAMERA, > > + .selector = UVC_CT_PANTILT_RELATIVE_CONTROL, > > + .size = 16, > > + .offset = 0, > > + .v4l2_type = V4L2_CTRL_TYPE_INTEGER, > > + .data_type = UVC_CTRL_DATA_TYPE_SIGNED, > > + .get= uvc_ctrl_get_rel_speed, > > + .set= uvc_ctrl_set_rel_speed, > > + }, > > + { > > + .id = V4L2_CID_TILT_SPEED, > > + .name = "Tilt (Speed)", > > + .entity = UVC_GUID_UVC_CAMERA, > > + .selector = UVC_CT_PANTILT_RELATIVE_CONTROL, > > + .size = 16, > > + .offset = 16, > > + .v4l2_type = V4L2_CTRL_TYPE_INTEGER, > > + .data_type = UVC_CTRL_DATA_TYPE_SIGNED, > > + .get= uvc_ctrl_get_rel_speed, > > + .set= uvc_ctrl_set_rel_speed, > > + }, > > + { > > .id = V4L2_CID_PRIVACY, > > .name = "Privacy", > > .entity = UVC_GUID_UVC_CAMERA, > > -- > 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 v5 1/2] [media] V4L: Add camera pan/tilt speed controls
The V4L2_CID_PAN_SPEED and V4L2_CID_TILT_SPEED controls allow to move the camera by setting its rotation speed around its axis. Signed-off-by: Vincent Palatin Reviewed-by: Pawel Osciak --- Changes from v1: - update the documentation wording according to Pawel suggestion. Changes from v2: - bump Linux kernel version for the API change. Documentation/DocBook/media/v4l/compat.xml | 10 ++ Documentation/DocBook/media/v4l/controls.xml | 21 + drivers/media/v4l2-core/v4l2-ctrls.c | 2 ++ include/uapi/linux/v4l2-controls.h | 2 ++ 4 files changed, 35 insertions(+) diff --git a/Documentation/DocBook/media/v4l/compat.xml b/Documentation/DocBook/media/v4l/compat.xml index 3a626d1..0a2debf 100644 --- a/Documentation/DocBook/media/v4l/compat.xml +++ b/Documentation/DocBook/media/v4l/compat.xml @@ -2569,6 +2569,16 @@ fields changed from _s32 to _u32. + + V4L2 in Linux 3.18 + + + Added V4L2_CID_PAN_SPEED and + V4L2_CID_TILT_SPEED camera controls. + + + + Relation of V4L2 to other Linux multimedia APIs diff --git a/Documentation/DocBook/media/v4l/controls.xml b/Documentation/DocBook/media/v4l/controls.xml index 9f5ffd8..124f287 100644 --- a/Documentation/DocBook/media/v4l/controls.xml +++ b/Documentation/DocBook/media/v4l/controls.xml @@ -3965,6 +3965,27 @@ by exposure, white balance or focus controls. + + V4L2_CID_PAN_SPEED + integer + This control turns the +camera horizontally at the specific speed. The unit is undefined. A +positive value moves the camera to the right (clockwise when viewed +from above), a negative value to the left. A value of zero stops the motion +if one is in progress and has no effect otherwise. + + + + + V4L2_CID_TILT_SPEED + integer + This control turns the +camera vertically at the specified speed. The unit is undefined. A +positive value moves the camera up, a negative value down. A value of zero +stops the motion if one is in progress and has no effect otherwise. + + + diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c b/drivers/media/v4l2-core/v4l2-ctrls.c index f030d6a..4d050f9 100644 --- a/drivers/media/v4l2-core/v4l2-ctrls.c +++ b/drivers/media/v4l2-core/v4l2-ctrls.c @@ -796,6 +796,8 @@ const char *v4l2_ctrl_get_name(u32 id) case V4L2_CID_AUTO_FOCUS_STOP: return "Auto Focus, Stop"; case V4L2_CID_AUTO_FOCUS_STATUS:return "Auto Focus, Status"; case V4L2_CID_AUTO_FOCUS_RANGE: return "Auto Focus, Range"; + case V4L2_CID_PAN_SPEED:return "Pan, Speed"; + case V4L2_CID_TILT_SPEED: return "Tilt, Speed"; /* FM Radio Modulator controls */ /* Keep the order of the 'case's the same as in v4l2-controls.h! */ diff --git a/include/uapi/linux/v4l2-controls.h b/include/uapi/linux/v4l2-controls.h index e946e43..4de238b 100644 --- a/include/uapi/linux/v4l2-controls.h +++ b/include/uapi/linux/v4l2-controls.h @@ -746,6 +746,8 @@ enum v4l2_auto_focus_range { V4L2_AUTO_FOCUS_RANGE_INFINITY = 3, }; +#define V4L2_CID_PAN_SPEED (V4L2_CID_CAMERA_CLASS_BASE+32) +#define V4L2_CID_TILT_SPEED(V4L2_CID_CAMERA_CLASS_BASE+33) /* FM Modulator class control IDs */ -- 2.1.0.rc2.206.gedb03e5 -- 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 v5 2/2] V4L: uvcvideo: Add support for pan/tilt speed controls
Map V4L2_CID_TILT_SPEED and V4L2_CID_PAN_SPEED to the standard UVC CT_PANTILT_RELATIVE_CONTROL terminal control request. Tested by plugging a Logitech ConferenceCam C3000e USB camera and controlling pan/tilt from the userspace using the VIDIOC_S_CTRL ioctl. Verified that it can pan and tilt at the same time in both directions. Signed-off-by: Vincent Palatin Reviewed-by: Pawel Osciak --- Changes from v1/v2: - rebased Changes from v3: - removed gerrit-id Chnages from v4: - switched "offset" to unsigned int drivers/media/usb/uvc/uvc_ctrl.c | 58 +--- 1 file changed, 55 insertions(+), 3 deletions(-) diff --git a/drivers/media/usb/uvc/uvc_ctrl.c b/drivers/media/usb/uvc/uvc_ctrl.c index 0eb82106..d2d1755 100644 --- a/drivers/media/usb/uvc/uvc_ctrl.c +++ b/drivers/media/usb/uvc/uvc_ctrl.c @@ -309,9 +309,8 @@ static struct uvc_control_info uvc_ctrls[] = { .selector = UVC_CT_PANTILT_RELATIVE_CONTROL, .index = 12, .size = 4, - .flags = UVC_CTRL_FLAG_SET_CUR | UVC_CTRL_FLAG_GET_MIN - | UVC_CTRL_FLAG_GET_MAX | UVC_CTRL_FLAG_GET_RES - | UVC_CTRL_FLAG_GET_DEF + .flags = UVC_CTRL_FLAG_SET_CUR + | UVC_CTRL_FLAG_GET_RANGE | UVC_CTRL_FLAG_AUTO_UPDATE, }, { @@ -391,6 +390,35 @@ static void uvc_ctrl_set_zoom(struct uvc_control_mapping *mapping, data[2] = min((int)abs(value), 0xff); } +static __s32 uvc_ctrl_get_rel_speed(struct uvc_control_mapping *mapping, + __u8 query, const __u8 *data) +{ + unsigned int first = mapping->offset / 8; + __s8 rel = (__s8)data[first]; + + switch (query) { + case UVC_GET_CUR: + return (rel == 0) ? 0 : (rel > 0 ? data[first+1] +: -data[first+1]); + case UVC_GET_MIN: + return -data[first+1]; + case UVC_GET_MAX: + case UVC_GET_RES: + case UVC_GET_DEF: + default: + return data[first+1]; + } +} + +static void uvc_ctrl_set_rel_speed(struct uvc_control_mapping *mapping, + __s32 value, __u8 *data) +{ + unsigned int first = mapping->offset / 8; + + data[first] = value == 0 ? 0 : (value > 0) ? 1 : 0xff; + data[first+1] = min_t(int, abs(value), 0xff); +} + static struct uvc_control_mapping uvc_ctrl_mappings[] = { { .id = V4L2_CID_BRIGHTNESS, @@ -677,6 +705,30 @@ static struct uvc_control_mapping uvc_ctrl_mappings[] = { .data_type = UVC_CTRL_DATA_TYPE_SIGNED, }, { + .id = V4L2_CID_PAN_SPEED, + .name = "Pan (Speed)", + .entity = UVC_GUID_UVC_CAMERA, + .selector = UVC_CT_PANTILT_RELATIVE_CONTROL, + .size = 16, + .offset = 0, + .v4l2_type = V4L2_CTRL_TYPE_INTEGER, + .data_type = UVC_CTRL_DATA_TYPE_SIGNED, + .get= uvc_ctrl_get_rel_speed, + .set= uvc_ctrl_set_rel_speed, + }, + { + .id = V4L2_CID_TILT_SPEED, + .name = "Tilt (Speed)", + .entity = UVC_GUID_UVC_CAMERA, + .selector = UVC_CT_PANTILT_RELATIVE_CONTROL, + .size = 16, + .offset = 16, + .v4l2_type = V4L2_CTRL_TYPE_INTEGER, + .data_type = UVC_CTRL_DATA_TYPE_SIGNED, + .get= uvc_ctrl_get_rel_speed, + .set= uvc_ctrl_set_rel_speed, + }, + { .id = V4L2_CID_PRIVACY, .name = "Privacy", .entity = UVC_GUID_UVC_CAMERA, -- 2.1.0.rc2.206.gedb03e5 -- 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 2/2] V4L: uvcvideo: Add support for pan/tilt speed controls
On Tue, Oct 7, 2014 at 9:36 AM, Laurent Pinchart wrote: > > Hi Vincent, > > Thank you for the patch. Mauro has already merged this in his tree, it should > appear in v3.18-rc1. > Great ! I missed the merge. Thanks, -- Vincent > > On Monday 06 October 2014 14:05:59 Vincent Palatin wrote: > > Map V4L2_CID_TILT_SPEED and V4L2_CID_PAN_SPEED to the standard UVC > > CT_PANTILT_RELATIVE_CONTROL terminal control request. > > > > Tested by plugging a Logitech ConferenceCam C3000e USB camera > > and controlling pan/tilt from the userspace using the VIDIOC_S_CTRL ioctl. > > Verified that it can pan and tilt at the same time in both directions. > > > > Signed-off-by: Vincent Palatin > > Reviewed-by: Pawel Osciak > > --- > > Changes from v1/v2: > > - rebased > > Changes from v3: > > - removed gerrit-id > > Chnages from v4: > > - switched "offset" to unsigned int > > > > drivers/media/usb/uvc/uvc_ctrl.c | 58 ++--- > > 1 file changed, 55 insertions(+), 3 deletions(-) > > > > diff --git a/drivers/media/usb/uvc/uvc_ctrl.c > > b/drivers/media/usb/uvc/uvc_ctrl.c index 0eb82106..d2d1755 100644 > > --- a/drivers/media/usb/uvc/uvc_ctrl.c > > +++ b/drivers/media/usb/uvc/uvc_ctrl.c > > @@ -309,9 +309,8 @@ static struct uvc_control_info uvc_ctrls[] = { > > .selector = UVC_CT_PANTILT_RELATIVE_CONTROL, > > .index = 12, > > .size = 4, > > - .flags = UVC_CTRL_FLAG_SET_CUR | > > UVC_CTRL_FLAG_GET_MIN > > - | UVC_CTRL_FLAG_GET_MAX | > > UVC_CTRL_FLAG_GET_RES > > - | UVC_CTRL_FLAG_GET_DEF > > + .flags = UVC_CTRL_FLAG_SET_CUR > > + | UVC_CTRL_FLAG_GET_RANGE > > > > | UVC_CTRL_FLAG_AUTO_UPDATE, > > > > }, > > { > > @@ -391,6 +390,35 @@ static void uvc_ctrl_set_zoom(struct > > uvc_control_mapping *mapping, data[2] = min((int)abs(value), 0xff); > > } > > > > +static __s32 uvc_ctrl_get_rel_speed(struct uvc_control_mapping *mapping, > > + __u8 query, const __u8 *data) > > +{ > > + unsigned int first = mapping->offset / 8; > > + __s8 rel = (__s8)data[first]; > > + > > + switch (query) { > > + case UVC_GET_CUR: > > + return (rel == 0) ? 0 : (rel > 0 ? data[first+1] > > + : -data[first+1]); > > + case UVC_GET_MIN: > > + return -data[first+1]; > > + case UVC_GET_MAX: > > + case UVC_GET_RES: > > + case UVC_GET_DEF: > > + default: > > + return data[first+1]; > > + } > > +} > > + > > +static void uvc_ctrl_set_rel_speed(struct uvc_control_mapping *mapping, > > + __s32 value, __u8 *data) > > +{ > > + unsigned int first = mapping->offset / 8; > > + > > + data[first] = value == 0 ? 0 : (value > 0) ? 1 : 0xff; > > + data[first+1] = min_t(int, abs(value), 0xff); > > +} > > + > > static struct uvc_control_mapping uvc_ctrl_mappings[] = { > > { > > .id = V4L2_CID_BRIGHTNESS, > > @@ -677,6 +705,30 @@ static struct uvc_control_mapping uvc_ctrl_mappings[] = > > { .data_type = UVC_CTRL_DATA_TYPE_SIGNED, > > }, > > { > > + .id = V4L2_CID_PAN_SPEED, > > + .name = "Pan (Speed)", > > + .entity = UVC_GUID_UVC_CAMERA, > > + .selector = UVC_CT_PANTILT_RELATIVE_CONTROL, > > + .size = 16, > > + .offset = 0, > > + .v4l2_type = V4L2_CTRL_TYPE_INTEGER, > > + .data_type = UVC_CTRL_DATA_TYPE_SIGNED, > > + .get= uvc_ctrl_get_rel_speed, > > + .set= uvc_ctrl_set_rel_speed, > > + }, > > + { > > + .id = V4L2_CID_TILT_SPEED, > > + .name = "Tilt (Speed)", > > + .entity = UVC_GUID_UVC_CAMERA, > > + .selector = UVC_CT_PANTILT_RELATIVE_CONTROL, > > + .size = 16, > > + .offset = 16, > > + .v4l2_type = V4L2_CTRL_TYPE_INTEGER, > > + .data_type = UVC_CTRL_DATA_TYPE_SIGNED, > > + .get= uvc_ctrl_get_rel_speed, > > + .set= uvc_ctrl_set_rel_speed, > > + }, > > + { > > .id = V4L2_CID_PRIVACY, > > .name = "Privacy", > > .entity = UVC_GUID_UVC_CAMERA, > > -- > 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] DocBook: fix media build error
On Sun, Oct 19, 2014 at 7:39 PM, Randy Dunlap wrote: > From: Randy Dunlap > > Fix media DocBook build errors by making the orderedlist balanced. > > DOC1/Documentation/DocBook/compat.xml:2576: parser error : Opening and ending > tag mismatch: orderedlist line 2560 and section > DOC1/Documentation/DocBook/compat.xml:2726: parser error : Premature end of > data in tag section line 884 > DOC1/Documentation/DocBook/compat.xml:2726: parser error : chunk is not well > balanced > > Signed-off-by: Randy Dunlap > Cc: Vincent Palatin > --- > Documentation/DocBook/media/v4l/compat.xml |1 + > 1 file changed, 1 insertion(+) > > --- lnx-318-rc1.orig/Documentation/DocBook/media/v4l/compat.xml > +++ lnx-318-rc1/Documentation/DocBook/media/v4l/compat.xml > @@ -2566,6 +2566,7 @@ fields changed from _s32 to _u32. > Added compound control types and &VIDIOC-QUERY-EXT-CTRL;. > > > + >V4L2 in Linux 3.18 > > Compared to the original patch, it's actually also missing the which were lost in the merge. -- Vincent -- 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] DocBook: fix media build error
On Mon, Oct 20, 2014 at 10:08 AM, Randy Dunlap wrote: > On 10/20/14 09:06, Vincent Palatin wrote: >> On Sun, Oct 19, 2014 at 7:39 PM, Randy Dunlap wrote: >>> From: Randy Dunlap >>> >>> Fix media DocBook build errors by making the orderedlist balanced. >>> >>> DOC1/Documentation/DocBook/compat.xml:2576: parser error : Opening and >>> ending tag mismatch: orderedlist line 2560 and section >>> DOC1/Documentation/DocBook/compat.xml:2726: parser error : Premature end of >>> data in tag section line 884 >>> DOC1/Documentation/DocBook/compat.xml:2726: parser error : chunk is not >>> well balanced >>> >>> Signed-off-by: Randy Dunlap >>> Cc: Vincent Palatin >>> --- >>> Documentation/DocBook/media/v4l/compat.xml |1 + >>> 1 file changed, 1 insertion(+) >>> >>> --- lnx-318-rc1.orig/Documentation/DocBook/media/v4l/compat.xml >>> +++ lnx-318-rc1/Documentation/DocBook/media/v4l/compat.xml >>> @@ -2566,6 +2566,7 @@ fields changed from _s32 to _u32. >>> Added compound control types and &VIDIOC-QUERY-EXT-CTRL;. >>> >>> >>> + >>>V4L2 in Linux 3.18 >>> >>> >> >> Compared to the original patch, it's actually also missing the >> >> >> >> which were lost in the merge. >> >> > > Would you please send a complete fix for it? > then Mauro can drop my patch. Sure, I will send it in 5 minutes, the docs are currently rebuilding with the patch ... -- Vincent -- 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] [media] v4l: DocBook: fix media build error
Fix media DocBook build errors by re-adding the orderedlist tag and putting back the section tags lost during merge. Signed-off-by: Vincent Palatin --- Documentation/DocBook/media/v4l/compat.xml | 4 1 file changed, 4 insertions(+) diff --git a/Documentation/DocBook/media/v4l/compat.xml b/Documentation/DocBook/media/v4l/compat.xml index 07ffc76..0a2debf 100644 --- a/Documentation/DocBook/media/v4l/compat.xml +++ b/Documentation/DocBook/media/v4l/compat.xml @@ -2566,6 +2566,10 @@ fields changed from _s32 to _u32. Added compound control types and &VIDIOC-QUERY-EXT-CTRL;. + + + + V4L2 in Linux 3.18 -- 2.1.0.rc2.206.gedb03e5 -- 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 RESEND] usb: dwc2: avoid leaking DMA channels on disconnection
When the HCD is disconnected, the DMA transfers still in-flight were cleaned-up but the count of available DMA channels (e.g. available_host_channels) was not reset. The pool of DMA channels can be depleted when doing unclean disconnection of USB peripherals, and reaches the point where no transfer was possible until the next reboot/reload of the driver. Tested by putting a programmable USB mux on the port and randomly plugging/unpluging a USB HUB with USB mass-storage key, USB-audio and USB-ethernet dongle connected to its downstream ports, and also doing the disconnection early while the devices are still enumerating to get more URBs in-flight. After the patch, the devices are still enumerating after thousands of cycles, while the port was totally dead before. Signed-off-by: Vincent Palatin --- I'm re-sending it, it seems the previous email did not show up. drivers/usb/dwc2/hcd.c | 8 1 file changed, 8 insertions(+) diff --git a/drivers/usb/dwc2/hcd.c b/drivers/usb/dwc2/hcd.c index c78c874..559b55e 100644 --- a/drivers/usb/dwc2/hcd.c +++ b/drivers/usb/dwc2/hcd.c @@ -257,6 +257,14 @@ static void dwc2_hcd_cleanup_channels(struct dwc2_hsotg *hsotg) */ channel->qh = NULL; } + /* All channels have been freed, mark them available */ + if (hsotg->core_params->uframe_sched > 0) { + hsotg->available_host_channels = + hsotg->core_params->host_channels; + } else { + hsotg->non_periodic_channels = 0; + hsotg->periodic_channels = 0; + } } /** -- 2.2.0.rc0.207.ga3a616c -- 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] usb: dwc2: avoid leaking DMA channels on disconnection
When the HCD is disconnected, the DMA transfers still in-flight were cleaned-up but the count of available DMA channels (e.g. available_host_channels) was not reset. The pool of DMA channels can be depleted when doing unclean disconnection of USB peripherals, and reaches the point where no transfer was possible until the next reboot/reload of the driver. Tested by putting a programmable USB mux on the port and randomly plugging/unpluging a USB HUB with USB mass-storage key, USB-audio and USB-ethernet dongle connected to its downstream ports, and also doing the disconnection early while the devices are still enumerating to get more URBs in-flight. After the patch, the devices are still enumerating after thousands of cycles, while the port was totally dead before. Signed-off-by: Vincent Palatin --- drivers/usb/dwc2/hcd.c | 8 1 file changed, 8 insertions(+) diff --git a/drivers/usb/dwc2/hcd.c b/drivers/usb/dwc2/hcd.c index c78c874..559b55e 100644 --- a/drivers/usb/dwc2/hcd.c +++ b/drivers/usb/dwc2/hcd.c @@ -257,6 +257,14 @@ static void dwc2_hcd_cleanup_channels(struct dwc2_hsotg *hsotg) */ channel->qh = NULL; } + /* All channels have been freed, mark them available */ + if (hsotg->core_params->uframe_sched > 0) { + hsotg->available_host_channels = + hsotg->core_params->host_channels; + } else { + hsotg->non_periodic_channels = 0; + hsotg->periodic_channels = 0; + } } /** -- 2.2.0.rc0.207.ga3a616c -- 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] net: stmmac: dwmac-rk: keep PHY up for WoL
On Tue, Jun 7, 2016 at 12:23 AM, Giuseppe CAVALLARO wrote: > Hello > > On 6/3/2016 7:29 PM, Vincent Palatin wrote: >> >> Do not shutdown the PHY if Wake-on-Lan is enabled, else it cannot wake >> us up. >> > > I do not understand why you need that. > This is done inside the PHY layer and it is tested on our platforms > he idea is: If the parent wants to Wake the system then the PHY should > not power-down. I'm not sure I understand : you mean that this path is not called if WoL is enabled ? [ currently stmmac_pltfr_suspend() is calling priv->plat->exit() which is the rk_gmac_exit() code I'm modifying ] or the RK driver code should not power down the phy in its exit() callback ? >> Signed-off-by: Vincent Palatin >> --- >> drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c | 8 >> 1 file changed, 8 insertions(+) >> >> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c >> b/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c >> index 0cd3ecf..2e45e75 100644 >> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c >> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c >> @@ -534,6 +534,10 @@ static int rk_gmac_init(struct platform_device *pdev, >> void *priv) >> struct rk_priv_data *bsp_priv = priv; >> int ret; >> >> + /* Keep the PHY up if we use Wake-on-Lan. */ >> + if (device_may_wakeup(&pdev->dev)) >> + return 0; >> + >> ret = phy_power_on(bsp_priv, true); >> if (ret) >> return ret; >> @@ -549,6 +553,10 @@ static void rk_gmac_exit(struct platform_device >> *pdev, void *priv) >> { >> struct rk_priv_data *gmac = priv; >> >> + /* The PHY was up for Wake-on-Lan. */ >> + if (device_may_wakeup(&pdev->dev)) >> + return; >> + >> phy_power_on(gmac, false); >> gmac_clk_enable(gmac, false); >> } >> >
Re: [PATCH] net: stmmac: dwmac-rk: keep PHY up for WoL
On Wed, Jun 8, 2016 at 5:17 PM, Andrew Lunn wrote: > On Wed, Jun 08, 2016 at 03:25:38PM -0700, Vincent Palatin wrote: >> On Tue, Jun 7, 2016 at 12:23 AM, Giuseppe CAVALLARO >> wrote: >> > Hello >> > >> > On 6/3/2016 7:29 PM, Vincent Palatin wrote: >> >> >> >> Do not shutdown the PHY if Wake-on-Lan is enabled, else it cannot wake >> >> us up. >> >> >> > >> > I do not understand why you need that. >> > This is done inside the PHY layer and it is tested on our platforms >> > he idea is: If the parent wants to Wake the system then the PHY should >> > not power-down. >> >> I'm not sure I understand : >> you mean that this path is not called if WoL is enabled ? >> [ currently stmmac_pltfr_suspend() is calling priv->plat->exit() which >> is the rk_gmac_exit() code I'm modifying ] >> or the RK driver code should not power down the phy in its exit() callback ? > > Take a look at phy_suspend(). phy_suspend() sends (or not) the PowerDown command to the PHY through the MDIO bus, depending if WoL is disabled, but most of my question still stands as far as I can tell : I was trying to get a proper WoL support on the following setup : dwmac (inside a RK3288 SoC) connected to RTL8211 PHY The current upstream code for this case will call rk_gmac_exit() when the MAC suspends (after the PHY has already suspended). Effectively doing a phy_power_on(, false) which is calling regulator_disable() on the LDO defined by the 'phy-supply' attribute. So my reading is that the RK specific MAC code is turning off unconditionally the PHY power regulator. Unless I'm mistaken, either this code is incorrect for the WoL case or the naming 'phy-supply' is misleading and should be the MAC supply. -- Vincent
Re: [PATCH 2/3] net: stmmac: dwmac-rk: keep the PHY up for WoL
On Fri, Jun 10, 2016 at 6:57 PM, Heiko Stuebner wrote: > Am Freitag, 10. Juni 2016, 18:00:38 schrieb Vincent Palatin: >> When suspending the machine, do not shutdown the external PHY by cutting >> its regulator in the mac platform driver suspend code if Wake-on-Lan is >> enabled, else it cannot wake us up. >> In order to do this, split the suspend/resume callbacks from the >> init/exit callbacks, so we can condition the power-down on the lack of >> need to wake-up from the LAN but do it unconditionally when unloading the >> module. >> >> Signed-off-by: Vincent Palatin >> --- >> drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c | 49 >> +++--- 1 file changed, 44 insertions(+), 5 >> deletions(-) >> >> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c >> b/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c index 0cd3ecf..fa05771 >> 100644 >> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c >> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c >> @@ -46,6 +46,7 @@ struct rk_priv_data { >> struct platform_device *pdev; >> int phy_iface; >> struct regulator *regulator; >> + bool powered_down; >> const struct rk_gmac_ops *ops; >> >> bool clk_enabled; >> @@ -529,9 +530,8 @@ static struct rk_priv_data *rk_gmac_setup(struct >> platform_device *pdev, return bsp_priv; >> } >> >> -static int rk_gmac_init(struct platform_device *pdev, void *priv) >> +static int rk_gmac_powerup(struct rk_priv_data *bsp_priv) >> { >> - struct rk_priv_data *bsp_priv = priv; >> int ret; >> >> ret = phy_power_on(bsp_priv, true); >> @@ -542,15 +542,52 @@ static int rk_gmac_init(struct platform_device >> *pdev, void *priv) if (ret) >> return ret; >> >> + bsp_priv->powered_down = true; >> + >> return 0; >> } >> >> -static void rk_gmac_exit(struct platform_device *pdev, void *priv) >> +static void rk_gmac_powerdown(struct rk_priv_data *gmac) >> { >> - struct rk_priv_data *gmac = priv; >> - >> phy_power_on(gmac, false); >> gmac_clk_enable(gmac, false); >> + gmac->powered_down = true; > > naming it gmac->suspended and doing all accesses in the suspend/resume > callback might provide a nicer way? Now the check is in resume while the > powerdown callback is setting it. > >> +} >> + >> +static int rk_gmac_init(struct platform_device *pdev, void *priv) >> +{ >> + struct rk_priv_data *bsp_priv = priv; >> + >> + return rk_gmac_powerup(bsp_priv); >> +} >> + >> +static void rk_gmac_exit(struct platform_device *pdev, void *priv) >> +{ >> + struct rk_priv_data *bsp_priv = priv; >> + >> + rk_gmac_powerdown(bsp_priv); >> +} >> + >> +static void rk_gmac_suspend(struct platform_device *pdev, void *priv) >> +{ >> + struct rk_priv_data *bsp_priv = priv; >> + >> + /* Keep the PHY up if we use Wake-on-Lan. */ >> + if (device_may_wakeup(&pdev->dev)) >> + return; >> + >> + rk_gmac_powerdown(bsp_priv); > > aka do > bsp_priv->suspended = true; > here > >> +} >> + >> +static void rk_gmac_resume(struct platform_device *pdev, void *priv) >> +{ >> + struct rk_priv_data *bsp_priv = priv; >> + >> + /* The PHY was up for Wake-on-Lan. */ >> + if (!bsp_priv->powered_down) >> + return; >> + >> + rk_gmac_powerup(bsp_priv); > > missing something like > bsp_priv->suspended = false; > > Right now it looks like your bsp_priv->powered_down will always be true > after the first suspend with powerdown. Yes I screw up badly, that's a good reason to use a more sensible name for the variable. > >> } >> >> static void rk_fix_speed(void *priv, unsigned int speed) >> @@ -591,6 +628,8 @@ static int rk_gmac_probe(struct platform_device *pdev) >> plat_dat->init = rk_gmac_init; >> plat_dat->exit = rk_gmac_exit; >> plat_dat->fix_mac_speed = rk_fix_speed; >> + plat_dat->suspend = rk_gmac_suspend; >> + plat_dat->resume = rk_gmac_resume; >> >> plat_dat->bsp_priv = rk_gmac_setup(pdev, data); >> if (IS_ERR(plat_dat->bsp_priv)) >> -- >> 2.8.0.rc3.226.g39d4020 >
Re: net: stmmac: dwmac-rk: fixes for Wake-on-Lan on RK3288
On Sun, Jun 12, 2016 at 11:46 PM, Giuseppe CAVALLARO wrote: > On 6/11/2016 3:00 AM, Vincent Palatin wrote: >> >> In order to support Wake-On-Lan when using the RK3288 integrated MAC >> (with an external RGMII PHY), we need to avoid shutting down the regulator >> of the external PHY when the MAC is suspended as it's currently done in >> the MAC >> platform code. >> As a first step, create independant callbacks for suspend/resume rather >> than >> re-using exit/init callbacks. So the dwmac platform driver can behave >> differently >> on suspend where it might skip shutting the PHY and at module unloading. >> Then update the dwmac-rk driver to switch off the PHY regulator only if we >> are >> not planning to wake up from the LAN. >> Finally add the PMT interrupt to the MAC device tree configuration, so we >> can >> wake up the core from it when the PHY has received the magic packet. > > > IMO these could be sent for net-next and also other glue logic > files should be reworked in order to use the new API for coherence. Given they will have the same set of functions for exit/init and suspend/resume, you mean duplicating the callbacks like this : --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-sti.c +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-sti.c @@ -359,6 +359,8 @@ static int sti_dwmac_probe(struct platform_device *pdev) plat_dat->bsp_priv = dwmac; plat_dat->init = sti_dwmac_init; plat_dat->exit = sti_dwmac_exit; + plat_dat->suspend = sti_dwmac_exit; + plat_dat->resume = sti_dwmac_init; plat_dat->fix_mac_speed = data->fix_retime_src; ret = sti_dwmac_init(pdev, plat_dat->bsp_priv); Is this anyhow useful ? -- Vincent
[PATCH v2 0/3] net: stmmac: dwmac-rk: fixes for Wake-on-Lan on RK3288
In order to support Wake-On-Lan when using the RK3288 integrated MAC (with an external RGMII PHY), we need to avoid shutting down the regulator of the external PHY when the MAC is suspended as it's currently done in the MAC platform code. As a first step, create independant callbacks for suspend/resume rather than re-using exit/init callbacks. So the dwmac platform driver can behave differently on suspend where it might skip shutting the PHY and at module unloading. Then update the dwmac-rk driver to switch off the PHY regulator only if we are not planning to wake up from the LAN. Finally add the PMT interrupt to the MAC device tree configuration, so we can wake up the core from it when the PHY has received the magic packet. Changes since v1: * rename 'powered_down' variable into 'suspended'. * fix the logic recording the PHY suspended state according to Heiko comments. Vincent Palatin (3): net: stmmac: allow to split suspend/resume from init/exit callbacks net: stmmac: dwmac-rk: keep the PHY up for WoL ARM: dts: rockchip: add interrupt for Wake-on-Lan on RK3288 arch/arm/boot/dts/rk3288.dtsi | 5 ++- drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c | 48 +++--- .../net/ethernet/stmicro/stmmac/stmmac_platform.c | 8 +++- include/linux/stmmac.h | 2 + 4 files changed, 54 insertions(+), 9 deletions(-) -- 2.8.0.rc3.226.g39d4020
[PATCH v2 2/3] net: stmmac: dwmac-rk: keep the PHY up for WoL
When suspending the machine, do not shutdown the external PHY by cutting its regulator in the mac platform driver suspend code if Wake-on-Lan is enabled, else it cannot wake us up. In order to do this, split the suspend/resume callbacks from the init/exit callbacks, so we can condition the power-down on the lack of need to wake-up from the LAN but do it unconditionally when unloading the module. Signed-off-by: Vincent Palatin --- drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c | 48 +++--- 1 file changed, 43 insertions(+), 5 deletions(-) diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c index 0cd3ecf..63c2e4f 100644 --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c @@ -46,6 +46,7 @@ struct rk_priv_data { struct platform_device *pdev; int phy_iface; struct regulator *regulator; + bool suspended; const struct rk_gmac_ops *ops; bool clk_enabled; @@ -529,9 +530,8 @@ static struct rk_priv_data *rk_gmac_setup(struct platform_device *pdev, return bsp_priv; } -static int rk_gmac_init(struct platform_device *pdev, void *priv) +static int rk_gmac_powerup(struct rk_priv_data *bsp_priv) { - struct rk_priv_data *bsp_priv = priv; int ret; ret = phy_power_on(bsp_priv, true); @@ -545,14 +545,50 @@ static int rk_gmac_init(struct platform_device *pdev, void *priv) return 0; } -static void rk_gmac_exit(struct platform_device *pdev, void *priv) +static void rk_gmac_powerdown(struct rk_priv_data *gmac) { - struct rk_priv_data *gmac = priv; - phy_power_on(gmac, false); gmac_clk_enable(gmac, false); } +static int rk_gmac_init(struct platform_device *pdev, void *priv) +{ + struct rk_priv_data *bsp_priv = priv; + + return rk_gmac_powerup(bsp_priv); +} + +static void rk_gmac_exit(struct platform_device *pdev, void *priv) +{ + struct rk_priv_data *bsp_priv = priv; + + rk_gmac_powerdown(bsp_priv); +} + +static void rk_gmac_suspend(struct platform_device *pdev, void *priv) +{ + struct rk_priv_data *bsp_priv = priv; + + /* Keep the PHY up if we use Wake-on-Lan. */ + if (device_may_wakeup(&pdev->dev)) + return; + + rk_gmac_powerdown(bsp_priv); + bsp_priv->suspended = true; +} + +static void rk_gmac_resume(struct platform_device *pdev, void *priv) +{ + struct rk_priv_data *bsp_priv = priv; + + /* The PHY was up for Wake-on-Lan. */ + if (!bsp_priv->suspended) + return; + + rk_gmac_powerup(bsp_priv); + bsp_priv->suspended = false; +} + static void rk_fix_speed(void *priv, unsigned int speed) { struct rk_priv_data *bsp_priv = priv; @@ -591,6 +627,8 @@ static int rk_gmac_probe(struct platform_device *pdev) plat_dat->init = rk_gmac_init; plat_dat->exit = rk_gmac_exit; plat_dat->fix_mac_speed = rk_fix_speed; + plat_dat->suspend = rk_gmac_suspend; + plat_dat->resume = rk_gmac_resume; plat_dat->bsp_priv = rk_gmac_setup(pdev, data); if (IS_ERR(plat_dat->bsp_priv)) -- 2.8.0.rc3.226.g39d4020
[PATCH v2 1/3] net: stmmac: allow to split suspend/resume from init/exit callbacks
Let the stmmac platform drivers provide dedicated suspend and resume callbacks rather than always re-using the init and exits callbacks. If the driver does not provide the suspend or resume callback, we fall back to the old behavior trying to use exit or init. This allows a specific platform to perform only a partial power-down on suspend if Wake-on-Lan is enabled but always perform the full shutdown sequence if the module is unloaded. Signed-off-by: Vincent Palatin --- drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c | 8 ++-- include/linux/stmmac.h| 2 ++ 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c index 409db91..a96714d 100644 --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c @@ -411,7 +411,9 @@ static int stmmac_pltfr_suspend(struct device *dev) struct platform_device *pdev = to_platform_device(dev); ret = stmmac_suspend(dev); - if (priv->plat->exit) + if (priv->plat->suspend) + priv->plat->suspend(pdev, priv->plat->bsp_priv); + else if (priv->plat->exit) priv->plat->exit(pdev, priv->plat->bsp_priv); return ret; @@ -430,7 +432,9 @@ static int stmmac_pltfr_resume(struct device *dev) struct stmmac_priv *priv = netdev_priv(ndev); struct platform_device *pdev = to_platform_device(dev); - if (priv->plat->init) + if (priv->plat->resume) + priv->plat->resume(pdev, priv->plat->bsp_priv); + else if (priv->plat->init) priv->plat->init(pdev, priv->plat->bsp_priv); return stmmac_resume(dev); diff --git a/include/linux/stmmac.h b/include/linux/stmmac.h index ffdaca9..0507dbf 100644 --- a/include/linux/stmmac.h +++ b/include/linux/stmmac.h @@ -135,6 +135,8 @@ struct plat_stmmacenet_data { void (*bus_setup)(void __iomem *ioaddr); int (*init)(struct platform_device *pdev, void *priv); void (*exit)(struct platform_device *pdev, void *priv); + void (*suspend)(struct platform_device *pdev, void *priv); + void (*resume)(struct platform_device *pdev, void *priv); void *bsp_priv; struct stmmac_axi *axi; int has_gmac4; -- 2.8.0.rc3.226.g39d4020
[PATCH v2 3/3] ARM: dts: rockchip: add interrupt for Wake-on-Lan on RK3288
In order to use Wake-on-Lan on RK3288 integrated MAC, we need to wake-up the CPU on the PMT interrupt when the MAC and the PHY are in low power mode. Adding the interrupt declaration. Signed-off-by: Vincent Palatin --- arch/arm/boot/dts/rk3288.dtsi | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/arch/arm/boot/dts/rk3288.dtsi b/arch/arm/boot/dts/rk3288.dtsi index 3b44ef3..3ebee53 100644 --- a/arch/arm/boot/dts/rk3288.dtsi +++ b/arch/arm/boot/dts/rk3288.dtsi @@ -539,8 +539,9 @@ gmac: ethernet@ff29 { compatible = "rockchip,rk3288-gmac"; reg = <0xff29 0x1>; - interrupts = ; - interrupt-names = "macirq"; + interrupts = , + ; + interrupt-names = "macirq", "eth_wake_irq"; rockchip,grf = <&grf>; clocks = <&cru SCLK_MAC>, <&cru SCLK_MAC_RX>, <&cru SCLK_MAC_TX>, -- 2.8.0.rc3.226.g39d4020
[PATCH] stmmac: do not sleep in atomic context for mdio_reset
stmmac_mdio_reset() has been updated to use msleep rather udelay (as some PHY requires a one second delay there). It called from stmmac_resume() within the spin_lock_irqsave block atomic context triggering 'scheduling while atomic'. The stmmac_priv lock usage is not fully documented, but it seems to protect the access to the MAC registers / DMA structures rather than the MDIO bus or the PHY (which have separate locking), so we can push the spin_lock after the stmmac_mdio_reset call. Signed-off-by: Vincent Palatin --- drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 6 -- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c index eac45d0..a473c18 100644 --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c @@ -3450,8 +3450,6 @@ int stmmac_resume(struct device *dev) if (!netif_running(ndev)) return 0; - spin_lock_irqsave(&priv->lock, flags); - /* Power Down bit, into the PM register, is cleared * automatically as soon as a magic packet or a Wake-up frame * is received. Anyway, it's better to manually clear @@ -3459,7 +3457,9 @@ int stmmac_resume(struct device *dev) * from another devices (e.g. serial console). */ if (device_may_wakeup(priv->device)) { + spin_lock_irqsave(&priv->lock, flags); priv->hw->mac->pmt(priv->hw, 0); + spin_unlock_irqrestore(&priv->lock, flags); priv->irq_wake = 0; } else { pinctrl_pm_select_default_state(priv->device); @@ -3473,6 +3473,8 @@ int stmmac_resume(struct device *dev) netif_device_attach(ndev); + spin_lock_irqsave(&priv->lock, flags); + priv->cur_rx = 0; priv->dirty_rx = 0; priv->dirty_tx = 0; -- 2.8.0.rc3.226.g39d4020
Re: net: stmmac: dwmac-rk: fixes for Wake-on-Lan on RK3288
Hi Giuseppe, On Thu, Jun 16, 2016 at 6:37 AM, Giuseppe CAVALLARO wrote: > > Hi Vincent > > > On 6/15/2016 7:04 PM, Vincent Palatin wrote: >> >> On Sun, Jun 12, 2016 at 11:46 PM, Giuseppe CAVALLARO >> wrote: >>> >>> On 6/11/2016 3:00 AM, Vincent Palatin wrote: >>>> >>>> >>>> In order to support Wake-On-Lan when using the RK3288 integrated MAC >>>> (with an external RGMII PHY), we need to avoid shutting down the regulator >>>> of the external PHY when the MAC is suspended as it's currently done in >>>> the MAC >>>> platform code. >>>> As a first step, create independant callbacks for suspend/resume rather >>>> than >>>> re-using exit/init callbacks. So the dwmac platform driver can behave >>>> differently >>>> on suspend where it might skip shutting the PHY and at module unloading. >>>> Then update the dwmac-rk driver to switch off the PHY regulator only if we >>>> are >>>> not planning to wake up from the LAN. >>>> Finally add the PMT interrupt to the MAC device tree configuration, so we >>>> can >>>> wake up the core from it when the PHY has received the magic packet. >>> >>> >>> >>> IMO these could be sent for net-next and also other glue logic >>> files should be reworked in order to use the new API for coherence. >> >> >> Given they will have the same set of functions for exit/init and >> suspend/resume, you mean duplicating the callbacks like this : >> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-sti.c >> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-sti.c >> @@ -359,6 +359,8 @@ static int sti_dwmac_probe(struct platform_device *pdev) >> plat_dat->bsp_priv = dwmac; >> plat_dat->init = sti_dwmac_init; >> plat_dat->exit = sti_dwmac_exit; >> + plat_dat->suspend = sti_dwmac_exit; >> + plat_dat->resume = sti_dwmac_init; >> plat_dat->fix_mac_speed = data->fix_retime_src; >> >> ret = sti_dwmac_init(pdev, plat_dat->bsp_priv); >> >> Is this anyhow useful ? > > > I think this is mandatory otherwise you are not guaranteeing the PM > stuff working on the rest of the glue-logics (not only sti); because > init/exit calls won't be called anymore. As mentioned in the PATCH 1/3 description: "If the driver does not provide the suspend or resume callback, we fall back to the old behavior trying to use exit or init. [...]" ie. --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c @@ -411,7 +411,9 @@ static int stmmac_pltfr_suspend(struct device *dev) struct platform_device *pdev = to_platform_device(dev); ret = stmmac_suspend(dev); - if (priv->plat->exit) + if (priv->plat->suspend) + priv->plat->suspend(pdev, priv->plat->bsp_priv); + else if (priv->plat->exit) priv->plat->exit(pdev, priv->plat->bsp_priv); return ret; @@ -430,7 +432,9 @@ static int stmmac_pltfr_resume(struct device *dev) struct stmmac_priv *priv = netdev_priv(ndev); struct platform_device *pdev = to_platform_device(dev); - if (priv->plat->init) + if (priv->plat->resume) + priv->plat->resume(pdev, priv->plat->bsp_priv); + else if (priv->plat->init) priv->plat->init(pdev, priv->plat->bsp_priv); return stmmac_resume(dev); So I was under the impression that everything should continue working as before for drivers only providing init/exit, by falling back on calling ->exit() if there is no suspend() callback initialized for the PM calls. You think that won't work ? > > So I kindly ask you to > propagate the fix and send the V3. The implementation above is ok for > me. > > peppe >
Re: [PATCHv7 1/3] usb: USB Type-C connector class
On Mon, Aug 29, 2016 at 12:23 PM, Heikki Krogerus wrote: > > The purpose of USB Type-C connector class is to provide > unified interface for the user space to get the status and > basic information about USB Type-C connectors on a system, > control over data role swapping, and when the port supports > USB Power Delivery, also control over power role swapping > and Alternate Modes. > > Signed-off-by: Heikki Krogerus > --- > Documentation/ABI/testing/sysfs-class-typec | 202 + > Documentation/usb/typec.txt | 103 +++ > MAINTAINERS |9 + > drivers/usb/Kconfig |2 + > drivers/usb/Makefile|2 + > drivers/usb/typec/Kconfig |7 + > drivers/usb/typec/Makefile |1 + > drivers/usb/typec/typec.c | 1090 > +++ > include/linux/usb/typec.h | 260 +++ > 9 files changed, 1676 insertions(+) > create mode 100644 Documentation/ABI/testing/sysfs-class-typec > create mode 100644 Documentation/usb/typec.txt > create mode 100644 drivers/usb/typec/Kconfig > create mode 100644 drivers/usb/typec/Makefile > create mode 100644 drivers/usb/typec/typec.c > create mode 100644 include/linux/usb/typec.h > > diff --git a/Documentation/ABI/testing/sysfs-class-typec > b/Documentation/ABI/testing/sysfs-class-typec > new file mode 100644 > index 000..c09a559 > --- /dev/null > +++ b/Documentation/ABI/testing/sysfs-class-typec > @@ -0,0 +1,202 @@ > +USB Type-C port devices (eg. /sys/class/typec/usbc0/) > + > +What: /sys/class/typec//current_data_role > +Date: June 2016 > +Contact: Heikki Krogerus > +Description: > + The current USB data role the port is operating in. This > + attribute can be used for requesting data role swapping on the > + port. Swapping is only supported as an asynchronous operation > + and requires polling of the attribute in order to know the > + result, so successful write operation does not mean successful > + swap. > + > + Valid values: > + - host > + - device > + > +What: /sys/class/typec//current_power_role > +Date: June 2016 > +Contact: Heikki Krogerus > +Description: > + The current power role of the port. This attribute can be used > + to request power role swap on the port when the port supports > + USB Power Delivery. I would put the same wording about asynchronous swapping as the data role. > > + > + Valid values: > + - source > + - sink > + > +What: /sys/class/typec//current_vconn_role Isn't the cover letter saying you have changed this ? > > +Date: June 2016 > +Contact: Heikki Krogerus > +Description: > + Shows the current VCONN role of the port. This attribute can > be > + used to request VCONN role swap on the port when the port > + supports USB Power Delivery. > + > + Valid values are: > + - source > + - sink > + > +What: /sys/class/typec//power_operation_mode > +Date: June 2016 > +Contact: Heikki Krogerus > +Description: > + Shows the current power operational mode the port is in. > + > + Valid values: > + - USB - Normal power levels defined in USB specifications > + - BC1.2 - Power levels defined in Battery Charging > Specification > + v1.2 > + - USB Type-C 1.5A - Higher 1.5A current defined in USB Type-C > + specification. > + - USB Type-C 3.0A - Higher 3A current defined in USB Type-C > + specification. > +- USB Power Delivery - The voltages and currents defined in > USB > + Power Delivery specification > + > +What: /sys/class/typec//preferred_role > +Date: June 2016 > +Contact: Heikki Krogerus > +Description: > + The user space can notify the driver about the preferred role. > + It should be handled as enabling of Try.SRC or Try.SNK, as > + defined in USB Type-C specification, in the port drivers. By > + default there is no preferred role. > + > + Valid values: > + - host > + - device > + - For example "none" to remove preference (anything else > except > + "host" or "device") > + > +What: /sys/class/typec//supported_accessory_modes > +Date: June 2016 > +Contact: Heikki Krogerus > +Description: > + Lists the Accessory Modes, defined in the USB Type-C > +
Re: [PATCHv6 1/3] usb: USB Type-C connector class
On Mon, Aug 29, 2016 at 2:36 PM, Heikki Krogerus wrote: > The purpose of USB Type-C connector class is to provide > unified interface for the user space to get the status and > basic information about USB Type-C connectors on a system, > control over data role swapping, and when the port supports > USB Power Delivery, also control over power role swapping > and Alternate Modes. > > Signed-off-by: Heikki Krogerus Acked-by: Vincent Palatin > --- > Documentation/ABI/testing/sysfs-class-typec | 205 + > Documentation/usb/typec.txt | 103 +++ > MAINTAINERS |9 + > drivers/usb/Kconfig |2 + > drivers/usb/Makefile|2 + > drivers/usb/typec/Kconfig |7 + > drivers/usb/typec/Makefile |1 + > drivers/usb/typec/typec.c | 1091 > +++ > include/linux/usb/typec.h | 260 +++ > 9 files changed, 1680 insertions(+) > create mode 100644 Documentation/ABI/testing/sysfs-class-typec > create mode 100644 Documentation/usb/typec.txt > create mode 100644 drivers/usb/typec/Kconfig > create mode 100644 drivers/usb/typec/Makefile > create mode 100644 drivers/usb/typec/typec.c > create mode 100644 include/linux/usb/typec.h > > diff --git a/Documentation/ABI/testing/sysfs-class-typec > b/Documentation/ABI/testing/sysfs-class-typec > new file mode 100644 > index 000..53fdd11 > --- /dev/null > +++ b/Documentation/ABI/testing/sysfs-class-typec > @@ -0,0 +1,205 @@ > +USB Type-C port devices (eg. /sys/class/typec/usbc0/) > + > +What: /sys/class/typec//current_data_role > +Date: June 2016 > +Contact: Heikki Krogerus > +Description: > + The current USB data role the port is operating in. This > + attribute can be used for requesting data role swapping on the > + port. Swapping is only supported as an asynchronous operation > + and requires polling of the attribute in order to know the > + result, so successful write operation does not mean successful > + swap. > + > + Valid values: > + - host > + - device > + > +What: /sys/class/typec//current_power_role > +Date: June 2016 > +Contact: Heikki Krogerus > +Description: > + The current power role of the port. This attribute can be used > + to request power role swap on the port when the port supports > + USB Power Delivery. Swapping is only supported as an > + asynchronous operation and requires polling of the attribute > in > + order to know the result, so successful write operation does > not > + mean successful swap. > + > + Valid values: > + - source > + - sink > + > +What: /sys/class/typec//vconn_source > +Date: June 2016 > +Contact: Heikki Krogerus > +Description: > + Shows is the port VCONN Source. This attribute can be used to > + request VCONN swap to change the VCONN Source during > connection > + when both the port and the partner support USB Power Delivery. > + > + Valid values are: > + - 0 when the port is not the VCONN Source > + - 1 when the port is the VCONN Source > + > +What: /sys/class/typec//power_operation_mode > +Date: June 2016 > +Contact: Heikki Krogerus > +Description: > + Shows the current power operational mode the port is in. > + > + Valid values: > + - USB - Normal power levels defined in USB specifications > + - BC1.2 - Power levels defined in Battery Charging > Specification > + v1.2 > + - USB Type-C 1.5A - Higher 1.5A current defined in USB Type-C > + specification. > + - USB Type-C 3.0A - Higher 3A current defined in USB Type-C > + specification. > +- USB Power Delivery - The voltages and currents defined in > USB > + Power Delivery specification > + > +What: /sys/class/typec//preferred_role > +Date: June 2016 > +Contact: Heikki Krogerus > +Description: > + The user space can notify the driver about the preferred role. > + It should be handled as enabling of Try.SRC or Try.SNK, as > + defi
Re: [PATCH] net: stmmac: dwmac-rk: keep PHY up for WoL
On Mon, Jun 6, 2016 at 1:45 PM, Heiko Stübner wrote: > Hi, > > Am Freitag, 3. Juni 2016, 10:29:20 schrieb Vincent Palatin: >> Do not shutdown the PHY if Wake-on-Lan is enabled, else it cannot wake >> us up. >> >> Signed-off-by: Vincent Palatin >> --- >> drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c | 8 >> 1 file changed, 8 insertions(+) >> >> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c >> b/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c index 0cd3ecf..2e45e75 >> 100644 >> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c >> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c >> @@ -534,6 +534,10 @@ static int rk_gmac_init(struct platform_device *pdev, >> void *priv) struct rk_priv_data *bsp_priv = priv; >> int ret; >> >> + /* Keep the PHY up if we use Wake-on-Lan. */ >> + if (device_may_wakeup(&pdev->dev)) >> + return 0; >> + > > Hmm, this looks like it would also block the initial setup of clocks and phy? Yes, that's bad. Doug told me so but I forget to CC him on the previous submission. I will do another version. > platform_device + device struct are created before probe gets called, so > something could set the wakeup flag before the driver initially probes? The device tree 'wakeup' attribute likely does it. -- Vincent
net: stmmac: dwmac-rk: fixes for Wake-on-Lan on RK3288
In order to support Wake-On-Lan when using the RK3288 integrated MAC (with an external RGMII PHY), we need to avoid shutting down the regulator of the external PHY when the MAC is suspended as it's currently done in the MAC platform code. As a first step, create independant callbacks for suspend/resume rather than re-using exit/init callbacks. So the dwmac platform driver can behave differently on suspend where it might skip shutting the PHY and at module unloading. Then update the dwmac-rk driver to switch off the PHY regulator only if we are not planning to wake up from the LAN. Finally add the PMT interrupt to the MAC device tree configuration, so we can wake up the core from it when the PHY has received the magic packet.
[PATCH 1/3] net: stmmac: allow to split suspend/resume from init/exit callbacks
Let the stmmac platform drivers provide dedicated suspend and resume callbacks rather than always re-using the init and exits callbacks. If the driver does not provide the suspend or resume callback, we fall back to the old behavior trying to use exit or init. This allows a specific platform to perform only a partial power-down on suspend if Wake-on-Lan is enabled but always perform the full shutdown sequence if the module is unloaded. Signed-off-by: Vincent Palatin --- drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c | 8 ++-- include/linux/stmmac.h| 2 ++ 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c index 409db91..a96714d 100644 --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c @@ -411,7 +411,9 @@ static int stmmac_pltfr_suspend(struct device *dev) struct platform_device *pdev = to_platform_device(dev); ret = stmmac_suspend(dev); - if (priv->plat->exit) + if (priv->plat->suspend) + priv->plat->suspend(pdev, priv->plat->bsp_priv); + else if (priv->plat->exit) priv->plat->exit(pdev, priv->plat->bsp_priv); return ret; @@ -430,7 +432,9 @@ static int stmmac_pltfr_resume(struct device *dev) struct stmmac_priv *priv = netdev_priv(ndev); struct platform_device *pdev = to_platform_device(dev); - if (priv->plat->init) + if (priv->plat->resume) + priv->plat->resume(pdev, priv->plat->bsp_priv); + else if (priv->plat->init) priv->plat->init(pdev, priv->plat->bsp_priv); return stmmac_resume(dev); diff --git a/include/linux/stmmac.h b/include/linux/stmmac.h index ffdaca9..0507dbf 100644 --- a/include/linux/stmmac.h +++ b/include/linux/stmmac.h @@ -135,6 +135,8 @@ struct plat_stmmacenet_data { void (*bus_setup)(void __iomem *ioaddr); int (*init)(struct platform_device *pdev, void *priv); void (*exit)(struct platform_device *pdev, void *priv); + void (*suspend)(struct platform_device *pdev, void *priv); + void (*resume)(struct platform_device *pdev, void *priv); void *bsp_priv; struct stmmac_axi *axi; int has_gmac4; -- 2.8.0.rc3.226.g39d4020
[PATCH 3/3] ARM: dts: rockchip: add interrupt for Wake-on-Lan on RK3288
In order to use Wake-on-Lan on RK3288 integrated MAC, we need to wake-up the CPU on the PMT interrupt when the MAC and the PHY are in low power mode. Adding the interrupt declaration. Signed-off-by: Vincent Palatin --- arch/arm/boot/dts/rk3288.dtsi | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/arch/arm/boot/dts/rk3288.dtsi b/arch/arm/boot/dts/rk3288.dtsi index 3b44ef3..3ebee53 100644 --- a/arch/arm/boot/dts/rk3288.dtsi +++ b/arch/arm/boot/dts/rk3288.dtsi @@ -539,8 +539,9 @@ gmac: ethernet@ff29 { compatible = "rockchip,rk3288-gmac"; reg = <0xff29 0x1>; - interrupts = ; - interrupt-names = "macirq"; + interrupts = , + ; + interrupt-names = "macirq", "eth_wake_irq"; rockchip,grf = <&grf>; clocks = <&cru SCLK_MAC>, <&cru SCLK_MAC_RX>, <&cru SCLK_MAC_TX>, -- 2.8.0.rc3.226.g39d4020
[PATCH 2/3] net: stmmac: dwmac-rk: keep the PHY up for WoL
When suspending the machine, do not shutdown the external PHY by cutting its regulator in the mac platform driver suspend code if Wake-on-Lan is enabled, else it cannot wake us up. In order to do this, split the suspend/resume callbacks from the init/exit callbacks, so we can condition the power-down on the lack of need to wake-up from the LAN but do it unconditionally when unloading the module. Signed-off-by: Vincent Palatin --- drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c | 49 +++--- 1 file changed, 44 insertions(+), 5 deletions(-) diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c index 0cd3ecf..fa05771 100644 --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c @@ -46,6 +46,7 @@ struct rk_priv_data { struct platform_device *pdev; int phy_iface; struct regulator *regulator; + bool powered_down; const struct rk_gmac_ops *ops; bool clk_enabled; @@ -529,9 +530,8 @@ static struct rk_priv_data *rk_gmac_setup(struct platform_device *pdev, return bsp_priv; } -static int rk_gmac_init(struct platform_device *pdev, void *priv) +static int rk_gmac_powerup(struct rk_priv_data *bsp_priv) { - struct rk_priv_data *bsp_priv = priv; int ret; ret = phy_power_on(bsp_priv, true); @@ -542,15 +542,52 @@ static int rk_gmac_init(struct platform_device *pdev, void *priv) if (ret) return ret; + bsp_priv->powered_down = true; + return 0; } -static void rk_gmac_exit(struct platform_device *pdev, void *priv) +static void rk_gmac_powerdown(struct rk_priv_data *gmac) { - struct rk_priv_data *gmac = priv; - phy_power_on(gmac, false); gmac_clk_enable(gmac, false); + gmac->powered_down = true; +} + +static int rk_gmac_init(struct platform_device *pdev, void *priv) +{ + struct rk_priv_data *bsp_priv = priv; + + return rk_gmac_powerup(bsp_priv); +} + +static void rk_gmac_exit(struct platform_device *pdev, void *priv) +{ + struct rk_priv_data *bsp_priv = priv; + + rk_gmac_powerdown(bsp_priv); +} + +static void rk_gmac_suspend(struct platform_device *pdev, void *priv) +{ + struct rk_priv_data *bsp_priv = priv; + + /* Keep the PHY up if we use Wake-on-Lan. */ + if (device_may_wakeup(&pdev->dev)) + return; + + rk_gmac_powerdown(bsp_priv); +} + +static void rk_gmac_resume(struct platform_device *pdev, void *priv) +{ + struct rk_priv_data *bsp_priv = priv; + + /* The PHY was up for Wake-on-Lan. */ + if (!bsp_priv->powered_down) + return; + + rk_gmac_powerup(bsp_priv); } static void rk_fix_speed(void *priv, unsigned int speed) @@ -591,6 +628,8 @@ static int rk_gmac_probe(struct platform_device *pdev) plat_dat->init = rk_gmac_init; plat_dat->exit = rk_gmac_exit; plat_dat->fix_mac_speed = rk_fix_speed; + plat_dat->suspend = rk_gmac_suspend; + plat_dat->resume = rk_gmac_resume; plat_dat->bsp_priv = rk_gmac_setup(pdev, data); if (IS_ERR(plat_dat->bsp_priv)) -- 2.8.0.rc3.226.g39d4020
[PATCH] net: stmmac: dwmac-rk: keep PHY up for WoL
Do not shutdown the PHY if Wake-on-Lan is enabled, else it cannot wake us up. Signed-off-by: Vincent Palatin --- drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c | 8 1 file changed, 8 insertions(+) diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c index 0cd3ecf..2e45e75 100644 --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c @@ -534,6 +534,10 @@ static int rk_gmac_init(struct platform_device *pdev, void *priv) struct rk_priv_data *bsp_priv = priv; int ret; + /* Keep the PHY up if we use Wake-on-Lan. */ + if (device_may_wakeup(&pdev->dev)) + return 0; + ret = phy_power_on(bsp_priv, true); if (ret) return ret; @@ -549,6 +553,10 @@ static void rk_gmac_exit(struct platform_device *pdev, void *priv) { struct rk_priv_data *gmac = priv; + /* The PHY was up for Wake-on-Lan. */ + if (device_may_wakeup(&pdev->dev)) + return; + phy_power_on(gmac, false); gmac_clk_enable(gmac, false); } -- 2.8.0.rc3.226.g39d4020
Re: [PATCH v2 2/8] mfd: cros_ec: free IRQ automatically
On Fri, Feb 23, 2018 at 4:05 PM, Enric Balletbo i Serra wrote: > > From: Vincent Palatin > > Free the IRQ we might have requested when removing the cros_ec device, > so we can unload and reload the driver properly. > > Signed-off-by: Vincent Palatin > Signed-off-by: Enric Balletbo i Serra > > --- > > Changes in v2: > - [2/8] Remove the free_irq in cros_ec_remove. > > drivers/mfd/cros_ec.c | 20 ++-- > 1 file changed, 6 insertions(+), 14 deletions(-) > > diff --git a/drivers/mfd/cros_ec.c b/drivers/mfd/cros_ec.c > index 74780f2964a1..ea8aa704b61e 100644 > --- a/drivers/mfd/cros_ec.c > +++ b/drivers/mfd/cros_ec.c > @@ -119,9 +119,9 @@ int cros_ec_register(struct cros_ec_device *ec_dev) > } > > if (ec_dev->irq) { > - err = request_threaded_irq(ec_dev->irq, NULL, ec_irq_thread, > - IRQF_TRIGGER_LOW | IRQF_ONESHOT, > - "chromeos-ec", ec_dev); > + err = devm_request_threaded_irq(dev, ec_dev->irq, NULL, > + ec_irq_thread, IRQF_TRIGGER_LOW | > IRQF_ONESHOT, > + "chromeos-ec", ec_dev); > if (err) { > dev_err(dev, "Failed to request IRQ %d: %d", > ec_dev->irq, err); > @@ -135,7 +135,7 @@ int cros_ec_register(struct cros_ec_device *ec_dev) > dev_err(dev, > "Failed to register Embedded Controller subdevice > %d\n", > err); > - goto fail_mfd; > + return err; > } > > if (ec_dev->max_passthru) { > @@ -153,7 +153,7 @@ int cros_ec_register(struct cros_ec_device *ec_dev) > dev_err(dev, > "Failed to register Power Delivery subdevice > %d\n", > err); > - goto fail_mfd; > + return err; > } > } > > @@ -162,7 +162,7 @@ int cros_ec_register(struct cros_ec_device *ec_dev) > if (err) { > mfd_remove_devices(dev); > dev_err(dev, "Failed to register sub-devices\n"); > - goto fail_mfd; > + return err; > } > } > > @@ -180,11 +180,6 @@ int cros_ec_register(struct cros_ec_device *ec_dev) > cros_ec_acpi_install_gpe_handler(dev); > > return 0; > - > -fail_mfd: > - if (ec_dev->irq) > - free_irq(ec_dev->irq, ec_dev); > - return err; > } > EXPORT_SYMBOL(cros_ec_register); > > @@ -194,9 +189,6 @@ int cros_ec_remove(struct cros_ec_device *ec_dev) > > cros_ec_acpi_remove_gpe_handler(); > > - if (ec_dev->irq) > - free_irq(ec_dev->irq, ec_dev); > - Acked-by: Vincent Palatin > > return 0; > } > EXPORT_SYMBOL(cros_ec_remove); > -- > 2.16.1 >
Re: [PATCH 2/6] mfd: cros_ec: free IRQ automatically
On Mon, Feb 19, 2018 at 11:40 PM, Enric Balletbo i Serra wrote: > From: Vincent Palatin > > Free the IRQ we might have requested when removing the cros_ec device, > so we can unload and reload the driver properly. > > Signed-off-by: Vincent Palatin > Signed-off-by: Enric Balletbo i Serra > --- > drivers/mfd/cros_ec.c | 17 ++--- > 1 file changed, 6 insertions(+), 11 deletions(-) > > diff --git a/drivers/mfd/cros_ec.c b/drivers/mfd/cros_ec.c > index 74780f2964a1..d1a4fbee9380 100644 > --- a/drivers/mfd/cros_ec.c > +++ b/drivers/mfd/cros_ec.c > @@ -119,9 +119,9 @@ int cros_ec_register(struct cros_ec_device *ec_dev) > } > > if (ec_dev->irq) { > - err = request_threaded_irq(ec_dev->irq, NULL, ec_irq_thread, > - IRQF_TRIGGER_LOW | IRQF_ONESHOT, > - "chromeos-ec", ec_dev); > + err = devm_request_threaded_irq(dev, ec_dev->irq, NULL, > + ec_irq_thread, IRQF_TRIGGER_LOW | > IRQF_ONESHOT, > + "chromeos-ec", ec_dev); > if (err) { > dev_err(dev, "Failed to request IRQ %d: %d", > ec_dev->irq, err); > @@ -135,7 +135,7 @@ int cros_ec_register(struct cros_ec_device *ec_dev) > dev_err(dev, > "Failed to register Embedded Controller subdevice > %d\n", > err); > - goto fail_mfd; > + return err; > } > > if (ec_dev->max_passthru) { > @@ -153,7 +153,7 @@ int cros_ec_register(struct cros_ec_device *ec_dev) > dev_err(dev, > "Failed to register Power Delivery subdevice > %d\n", > err); > - goto fail_mfd; > + return err; > } > } > > @@ -162,7 +162,7 @@ int cros_ec_register(struct cros_ec_device *ec_dev) > if (err) { > mfd_remove_devices(dev); > dev_err(dev, "Failed to register sub-devices\n"); > - goto fail_mfd; > + return err; > } > } > > @@ -180,11 +180,6 @@ int cros_ec_register(struct cros_ec_device *ec_dev) > cros_ec_acpi_install_gpe_handler(dev); > > return 0; > - > -fail_mfd: > - if (ec_dev->irq) > - free_irq(ec_dev->irq, ec_dev); > - return err; > } > EXPORT_SYMBOL(cros_ec_register); You need to remove the "free_irq(ec_dev->irq, ec_dev);" in cros_ec_remove() too. as done there: https://chromium-review.googlesource.com/c/chromiumos/third_party/kernel/+/866858/15/drivers/mfd/cros_ec.c it was missing originally but was added by: 'f58b14e6632a mfd: cros_ec: Free IRQ on exit' > > -- > 2.16.1 >
[tip:x86/urgent] x86, fpu: Avoid FPU lazy restore after suspend
Commit-ID: 644c154186386bb1fa6446bc5e037b9ed098db46 Gitweb: http://git.kernel.org/tip/644c154186386bb1fa6446bc5e037b9ed098db46 Author: Vincent Palatin AuthorDate: Fri, 30 Nov 2012 12:15:32 -0800 Committer: H. Peter Anvin CommitDate: Fri, 30 Nov 2012 13:48:05 -0800 x86, fpu: Avoid FPU lazy restore after suspend When a cpu enters S3 state, the FPU state is lost. After resuming for S3, if we try to lazy restore the FPU for a process running on the same CPU, this will result in a corrupted FPU context. Ensure that "fpu_owner_task" is properly invalided when (re-)initializing a CPU, so nobody will try to lazy restore a state which doesn't exist in the hardware. Tested with a 64-bit kernel on a 4-core Ivybridge CPU with eagerfpu=off, by doing thousands of suspend/resume cycles with 4 processes doing FPU operations running. Without the patch, a process is killed after a few hundreds cycles by a SIGFPE. Cc: Duncan Laurie Cc: Olof Johansson Cc: v3.4+ # for 3.4 need to replace this_cpu_write by percpu_write Signed-off-by: Vincent Palatin Link: http://lkml.kernel.org/r/1354306532-1014-1-git-send-email-vpala...@chromium.org Signed-off-by: H. Peter Anvin --- arch/x86/include/asm/fpu-internal.h | 15 +-- arch/x86/kernel/smpboot.c | 5 + 2 files changed, 14 insertions(+), 6 deletions(-) diff --git a/arch/x86/include/asm/fpu-internal.h b/arch/x86/include/asm/fpu-internal.h index 831dbb9..41ab26e 100644 --- a/arch/x86/include/asm/fpu-internal.h +++ b/arch/x86/include/asm/fpu-internal.h @@ -399,14 +399,17 @@ static inline void drop_init_fpu(struct task_struct *tsk) typedef struct { int preload; } fpu_switch_t; /* - * FIXME! We could do a totally lazy restore, but we need to - * add a per-cpu "this was the task that last touched the FPU - * on this CPU" variable, and the task needs to have a "I last - * touched the FPU on this CPU" and check them. + * Must be run with preemption disabled: this clears the fpu_owner_task, + * on this CPU. * - * We don't do that yet, so "fpu_lazy_restore()" always returns - * false, but some day.. + * This will disable any lazy FPU state restore of the current FPU state, + * but if the current thread owns the FPU, it will still be saved by. */ +static inline void __cpu_disable_lazy_restore(unsigned int cpu) +{ + per_cpu(fpu_owner_task, cpu) = NULL; +} + static inline int fpu_lazy_restore(struct task_struct *new, unsigned int cpu) { return new == this_cpu_read_stable(fpu_owner_task) && diff --git a/arch/x86/kernel/smpboot.c b/arch/x86/kernel/smpboot.c index c80a33b..f3e2ec8 100644 --- a/arch/x86/kernel/smpboot.c +++ b/arch/x86/kernel/smpboot.c @@ -68,6 +68,8 @@ #include #include #include +#include +#include #include #include #include @@ -818,6 +820,9 @@ int __cpuinit native_cpu_up(unsigned int cpu, struct task_struct *tidle) per_cpu(cpu_state, cpu) = CPU_UP_PREPARE; + /* the FPU context is blank, nobody can own it */ + __cpu_disable_lazy_restore(cpu); + err = do_boot_cpu(apicid, cpu, tidle); if (err) { pr_debug("do_boot_cpu failed %d\n", err); -- 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/