Re: [PATCH v2 2/3] usb: Add LED trigger for USB host activity
On Sat, Aug 23, 2014 at 2:52 AM, Michal Sojka wrote:
> Hi Bryan,
>
> thanks for the review. See some comments below.
>
> On Sat, Aug 23 2014, Bryan Wu wrote:
>> On Fri, Aug 22, 2014 at 5:08 PM, Michal Sojka wrote:
>>> With this patch, USB host activity can be signaled by blinking a LED.
>>>
>>> This should work with all host controllers. Tested only with musb.
>>>
>>> Signed-off-by: Michal Sojka
>>> ---
>>> drivers/usb/core/Kconfig | 9 +
>>> drivers/usb/core/Makefile | 1 +
>>> drivers/usb/core/hcd.c| 2 ++
>>> drivers/usb/core/led.c| 38 ++
>>> include/linux/usb/hcd.h | 6 ++
>>> 5 files changed, 56 insertions(+)
>>> create mode 100644 drivers/usb/core/led.c
>>>
>>> diff --git a/drivers/usb/core/Kconfig b/drivers/usb/core/Kconfig
>>> index 1060657..8295f65 100644
>>> --- a/drivers/usb/core/Kconfig
>>> +++ b/drivers/usb/core/Kconfig
>>> @@ -90,3 +90,12 @@ config USB_OTG_FSM
>>> Implements OTG Finite State Machine as specified in On-The-Go
>>> and Embedded Host Supplement to the USB Revision 2.0
>>> Specification.
>>>
>>> +config USB_HOST_LED
>>> + bool "USB Host LED Trigger"
>>> + depends on LEDS_CLASS
>>> + select LEDS_TRIGGERS
>>> + help
>>> + This option adds a LED trigger for USB host controllers.
>>> +
>>> + Say Y here if you are working on a system with led-class supported
>>> + LEDs and you want to use them as USB host activity indicators.
>>> diff --git a/drivers/usb/core/Makefile b/drivers/usb/core/Makefile
>>> index 2f6f932..324c8c9 100644
>>> --- a/drivers/usb/core/Makefile
>>> +++ b/drivers/usb/core/Makefile
>>> @@ -9,5 +9,6 @@ usbcore-y += port.o
>>>
>>> usbcore-$(CONFIG_PCI) += hcd-pci.o
>>> usbcore-$(CONFIG_ACPI) += usb-acpi.o
>>> +usbcore-$(CONFIG_USB_HOST_LED) += led.o
>>>
>>> obj-$(CONFIG_USB) += usbcore.o
>>> diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c
>>> index 487abcf..46d9f3a 100644
>>> --- a/drivers/usb/core/hcd.c
>>> +++ b/drivers/usb/core/hcd.c
>>> @@ -1664,6 +1664,8 @@ static void __usb_hcd_giveback_urb(struct urb *urb)
>>> usbmon_urb_complete(&hcd->self, urb, status);
>>> usb_anchor_suspend_wakeups(anchor);
>>> usb_unanchor_urb(urb);
>>> + if (status == 0)
>>> + usb_hcd_led_activity();
>>>
>>> /* pass ownership to the completion handler */
>>> urb->status = status;
>>> diff --git a/drivers/usb/core/led.c b/drivers/usb/core/led.c
>>> new file mode 100644
>>> index 000..49ff76c
>>> --- /dev/null
>>> +++ b/drivers/usb/core/led.c
>>> @@ -0,0 +1,38 @@
>>> +/*
>>> + * LED Trigger for USB Host Activity
>>> + *
>>> + * Copyright 2014 Michal Sojka
>>> + *
>>> + * This program is free software; you can redistribute it and/or modify
>>> + * it under the terms of the GNU General Public License version 2 as
>>> + * published by the Free Software Foundation.
>>> + *
>>> + */
>>> +
>>> +#include
>>> +#include
>>> +#include
>>> +#include
>>> +#include
>>> +
>>> +#define BLINK_DELAY 30
>>> +
>>> +DEFINE_LED_TRIGGER(ledtrig_usb_hcd);
>>
>> Add one more trigger named ledtrig_usb_gadget
>>
>>> +static unsigned long usb_hcd_blink_delay = BLINK_DELAY;
>>> +
>>> +void usb_hcd_led_activity(void)
>>
>> Give an input parameter like emum usb_led_event.
>> USB_LED_EVENT_HOST = 0
>> USB_LED_EVENT_GADGET = 1
>>
>>
>>> +{
>>
>> Add case for USB_LED_EVENT_HOST:
>>> + led_trigger_blink_oneshot(ledtrig_usb_hcd,
>>> + &usb_hcd_blink_delay,
>>> &usb_hcd_blink_delay, 0);
>>
>> Add case for USB_LED_EVENT_GADGET:
>> led_trigger_blink_oneshot(ledtrig_usb_gadget,
>> &usb_gadget_blink_delay,
>> &usb_gadget_blink_delay, 0);
>>
>>> +}
>>> +
>>> +int __init ledtrig_usb_hcd_init(void)
>>> +{
>>> + led_trigger_register_simple("usb-host", &ledtrig_usb_hcd);
>> register one more trigger for ga
Re: [PATCH v2 2/3] usb: Add LED trigger for USB host activity
On Fri, Aug 22, 2014 at 5:08 PM, Michal Sojka wrote:
> With this patch, USB host activity can be signaled by blinking a LED.
>
> This should work with all host controllers. Tested only with musb.
>
> Signed-off-by: Michal Sojka
> ---
> drivers/usb/core/Kconfig | 9 +
> drivers/usb/core/Makefile | 1 +
> drivers/usb/core/hcd.c| 2 ++
> drivers/usb/core/led.c| 38 ++
> include/linux/usb/hcd.h | 6 ++
> 5 files changed, 56 insertions(+)
> create mode 100644 drivers/usb/core/led.c
>
> diff --git a/drivers/usb/core/Kconfig b/drivers/usb/core/Kconfig
> index 1060657..8295f65 100644
> --- a/drivers/usb/core/Kconfig
> +++ b/drivers/usb/core/Kconfig
> @@ -90,3 +90,12 @@ config USB_OTG_FSM
> Implements OTG Finite State Machine as specified in On-The-Go
> and Embedded Host Supplement to the USB Revision 2.0 Specification.
>
> +config USB_HOST_LED
> + bool "USB Host LED Trigger"
> + depends on LEDS_CLASS
> + select LEDS_TRIGGERS
> + help
> + This option adds a LED trigger for USB host controllers.
> +
> + Say Y here if you are working on a system with led-class supported
> + LEDs and you want to use them as USB host activity indicators.
> diff --git a/drivers/usb/core/Makefile b/drivers/usb/core/Makefile
> index 2f6f932..324c8c9 100644
> --- a/drivers/usb/core/Makefile
> +++ b/drivers/usb/core/Makefile
> @@ -9,5 +9,6 @@ usbcore-y += port.o
>
> usbcore-$(CONFIG_PCI) += hcd-pci.o
> usbcore-$(CONFIG_ACPI) += usb-acpi.o
> +usbcore-$(CONFIG_USB_HOST_LED) += led.o
>
> obj-$(CONFIG_USB) += usbcore.o
> diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c
> index 487abcf..46d9f3a 100644
> --- a/drivers/usb/core/hcd.c
> +++ b/drivers/usb/core/hcd.c
> @@ -1664,6 +1664,8 @@ static void __usb_hcd_giveback_urb(struct urb *urb)
> usbmon_urb_complete(&hcd->self, urb, status);
> usb_anchor_suspend_wakeups(anchor);
> usb_unanchor_urb(urb);
> + if (status == 0)
> + usb_hcd_led_activity();
>
> /* pass ownership to the completion handler */
> urb->status = status;
> diff --git a/drivers/usb/core/led.c b/drivers/usb/core/led.c
> new file mode 100644
> index 000..49ff76c
> --- /dev/null
> +++ b/drivers/usb/core/led.c
> @@ -0,0 +1,38 @@
> +/*
> + * LED Trigger for USB Host Activity
> + *
> + * Copyright 2014 Michal Sojka
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + */
> +
> +#include
> +#include
> +#include
> +#include
> +#include
> +
> +#define BLINK_DELAY 30
> +
> +DEFINE_LED_TRIGGER(ledtrig_usb_hcd);
Add one more trigger named ledtrig_usb_gadget
> +static unsigned long usb_hcd_blink_delay = BLINK_DELAY;
> +
> +void usb_hcd_led_activity(void)
Give an input parameter like emum usb_led_event.
USB_LED_EVENT_HOST = 0
USB_LED_EVENT_GADGET = 1
> +{
Add case for USB_LED_EVENT_HOST:
> + led_trigger_blink_oneshot(ledtrig_usb_hcd,
> + &usb_hcd_blink_delay, &usb_hcd_blink_delay,
> 0);
Add case for USB_LED_EVENT_GADGET:
led_trigger_blink_oneshot(ledtrig_usb_gadget,
&usb_gadget_blink_delay,
&usb_gadget_blink_delay, 0);
> +}
> +
> +int __init ledtrig_usb_hcd_init(void)
> +{
> + led_trigger_register_simple("usb-host", &ledtrig_usb_hcd);
register one more trigger for gadget.
> + return 0;
> +}
> +
> +void __exit ledtrig_usb_hcd_exit(void)
> +{
> + led_trigger_unregister_simple(ledtrig_usb_hcd);
unregister one more trigger for gadget.
> +}
> diff --git a/include/linux/usb/hcd.h b/include/linux/usb/hcd.h
> index b43f0fe..eb5fa0f 100644
> --- a/include/linux/usb/hcd.h
> +++ b/include/linux/usb/hcd.h
> @@ -700,6 +700,12 @@ extern struct rw_semaphore ehci_cf_port_reset_rwsem;
> #define USB_EHCI_LOADED2
> extern unsigned long usb_hcds_loaded;
>
> +#ifdef CONFIG_USB_HOST_LED
> +extern void usb_hcd_led_activity(void);
User can call usb_led_activity(USB_LED_EVENT_HOST);
or usb_led_activity(USB_LED_EVENT_GADGET);
> +#else
> +static inline void usb_hcd_led_activity(void) {}
> +#endif
> +
> #endif /* __KERNEL__ */
>
> #endif /* __USB_CORE_HCD_H */
> --
> 2.1.0
>
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 0/3] LED triggers for USB host and device
On Fri, Aug 22, 2014 at 5:08 PM, Michal Sojka wrote: > This adds LED triggers for USB host and device. > > Changes from v1: > - Moved from drivers/leds/ to drivers/usb/ > - Improved Kconfig help > - Linked with other modules rather than being standalone modules > > Michal Sojka (3): > usb: Add missing #include > usb: Add LED trigger for USB host activity > usb: Add LED trigger for USB gadget activity > > drivers/usb/core/Kconfig| 9 + > drivers/usb/core/Makefile | 1 + > drivers/usb/core/hcd.c | 2 ++ > drivers/usb/core/led.c | 38 ++ I suggest put host trigger and gadget trigger together in drivers/usb/core/led.c. > drivers/usb/gadget/Kconfig | 12 > drivers/usb/gadget/udc/Makefile | 5 - > drivers/usb/gadget/udc/led.c| 38 ++ We don't need a duplicated version for gadget. > drivers/usb/musb/musb_gadget.c | 5 +++-- > include/linux/usb/gadget.h | 10 ++ > include/linux/usb/hcd.h | 7 +++ > 10 files changed, 124 insertions(+), 3 deletions(-) > create mode 100644 drivers/usb/core/led.c > create mode 100644 drivers/usb/gadget/udc/led.c > > -- > 2.1.0 > -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] leds: usb: Add LED trigger for USB gadget activity
On Fri, Aug 22, 2014 at 2:42 PM, Greg Kroah-Hartman wrote: > On Fri, Aug 22, 2014 at 10:39:03AM -0700, Bryan Wu wrote: >> On Fri, Aug 22, 2014 at 4:53 AM, Michal Sojka wrote: >> > With this patch, USB gadget activity can be signaled by blinking a LED. >> > >> > Since there is no generic code where to put the trigger for all USB >> > controllers, each USB controller needs to call the trigger individually. >> > This patch adds the call only for the musb controller where I can test >> > it. >> > >> >> Generally I think one led trigger for both USB host and USB gadget >> activity is good enough. We don't need 2 same led trigger here. > > What about systems that have both running at the same time? Don't you > want individual control? > Actually I wanted to say we don't need 2 same driver for USB host and USB gadget but one driver which has 2 led triggers like usb_host_ledtrig and usb_gadget_ledtrig. I think drivers/net/can/led.c is a good example to start. >> And probably you can just put this code in drivers/usb subsystem, >> since this driver is quite simple to add to USB subsystem. > > I have no objection to that, if the LED people don't mind it. > Because logically it's only used by USB subsystem and it can be a core component of USB, also drivers/net/can/led.c is a good example. Thanks, -Bryan -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] leds: usb: Add LED trigger for USB gadget activity
On Fri, Aug 22, 2014 at 4:53 AM, Michal Sojka wrote:
> With this patch, USB gadget activity can be signaled by blinking a LED.
>
> Since there is no generic code where to put the trigger for all USB
> controllers, each USB controller needs to call the trigger individually.
> This patch adds the call only for the musb controller where I can test
> it.
>
Generally I think one led trigger for both USB host and USB gadget
activity is good enough. We don't need 2 same led trigger here.
And probably you can just put this code in drivers/usb subsystem,
since this driver is quite simple to add to USB subsystem.
Thanks,
-Bryan
> Signed-off-by: Michal Sojka
> ---
> drivers/leds/trigger/Kconfig | 8 ++
> drivers/leds/trigger/Makefile| 1 +
> drivers/leds/trigger/ledtrig-usbgadget.c | 45
>
> drivers/usb/musb/musb_gadget.c | 6 +++--
> include/linux/leds.h | 6 +
> 5 files changed, 64 insertions(+), 2 deletions(-)
> create mode 100644 drivers/leds/trigger/ledtrig-usbgadget.c
>
> diff --git a/drivers/leds/trigger/Kconfig b/drivers/leds/trigger/Kconfig
> index 49794b4..9562963 100644
> --- a/drivers/leds/trigger/Kconfig
> +++ b/drivers/leds/trigger/Kconfig
> @@ -41,6 +41,14 @@ config LEDS_TRIGGER_IDE_DISK
> This allows LEDs to be controlled by IDE disk activity.
> If unsure, say Y.
>
> +config LEDS_TRIGGER_USBGADGET
> + bool "LED USB Gadget Trigger"
> + depends on (USB_MUSB_GADGET || USB_MUSB_DUAL_ROLE)
> + depends on LEDS_TRIGGERS
> + help
> + This allows LEDs to be controlled by USB gadget activity.
> + If unsure, say Y.
> +
> config LEDS_TRIGGER_HEARTBEAT
> tristate "LED Heartbeat Trigger"
> depends on LEDS_TRIGGERS
> diff --git a/drivers/leds/trigger/Makefile b/drivers/leds/trigger/Makefile
> index 1abf48d..45917c0 100644
> --- a/drivers/leds/trigger/Makefile
> +++ b/drivers/leds/trigger/Makefile
> @@ -8,3 +8,4 @@ obj-$(CONFIG_LEDS_TRIGGER_CPU) += ledtrig-cpu.o
> obj-$(CONFIG_LEDS_TRIGGER_DEFAULT_ON) += ledtrig-default-on.o
> obj-$(CONFIG_LEDS_TRIGGER_TRANSIENT) += ledtrig-transient.o
> obj-$(CONFIG_LEDS_TRIGGER_CAMERA) += ledtrig-camera.o
> +obj-$(CONFIG_LEDS_TRIGGER_USBGADGET) += ledtrig-usbgadget.o
> diff --git a/drivers/leds/trigger/ledtrig-usbgadget.c
> b/drivers/leds/trigger/ledtrig-usbgadget.c
> new file mode 100644
> index 000..1eb90da
> --- /dev/null
> +++ b/drivers/leds/trigger/ledtrig-usbgadget.c
> @@ -0,0 +1,45 @@
> +/*
> + * LED Trigger for USB Gadget Activity
> + *
> + * Copyright 2014 Michal Sojka
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + */
> +
> +#include
> +#include
> +#include
> +#include
> +
> +#define BLINK_DELAY 30
> +
> +DEFINE_LED_TRIGGER(ledtrig_usbgadget);
> +static unsigned long usbgadget_blink_delay = BLINK_DELAY;
> +
> +void ledtrig_usbgadget_activity(void)
> +{
> + led_trigger_blink_oneshot(ledtrig_usbgadget,
> + &usbgadget_blink_delay,
> &usbgadget_blink_delay, 0);
> +}
> +EXPORT_SYMBOL(ledtrig_usbgadget_activity);
> +
> +static int __init ledtrig_usbgadget_init(void)
> +{
> + led_trigger_register_simple("usb-gadget", &ledtrig_usbgadget);
> + return 0;
> +}
> +
> +static void __exit ledtrig_usbgadget_exit(void)
> +{
> + led_trigger_unregister_simple(ledtrig_usbgadget);
> +}
> +
> +module_init(ledtrig_usbgadget_init);
> +module_exit(ledtrig_usbgadget_exit);
> +
> +MODULE_AUTHOR("Michal Sojka ");
> +MODULE_DESCRIPTION("LED Trigger for USB Gadget Activity");
> +MODULE_LICENSE("GPL");
> diff --git a/drivers/usb/musb/musb_gadget.c b/drivers/usb/musb/musb_gadget.c
> index d4aa779..98f8b24 100644
> --- a/drivers/usb/musb/musb_gadget.c
> +++ b/drivers/usb/musb/musb_gadget.c
> @@ -42,6 +42,7 @@
> #include
> #include
> #include
> +#include
>
> #include "musb_core.h"
>
> @@ -167,11 +168,12 @@ __acquires(ep->musb->lock)
> if (!dma_mapping_error(&musb->g.dev, request->dma))
> unmap_dma_buffer(req, musb);
>
> - if (request->status == 0)
> + if (request->status == 0) {
> dev_dbg(musb->controller, "%s done request %p, %d/%d\n",
> ep->end_point.name, request,
> req->request.actual, req->request.length);
> - else
> + ledtrig_usbgadget_activity();
> + } else
> dev_dbg(musb->controller, "%s request %p, %d/%d fault %d\n",
> ep->end_point.name, request,
> req->request.actual, req->request.length,
> diff --git a/include/linux/leds.h b/include/linux/leds.h
> index 0287ab2..5d9668e 100644
> --- a/include/linux/leds.h
> +++ b/include/linux/leds.h
>
Re: [PATCH 2/2 v6] HID: gt683r: move mode attribute to led-class devices
On Thu, Jul 3, 2014 at 10:40 AM, Johan Hovold wrote:
> On Thu, Jul 03, 2014 at 08:17:09PM +0300, Janne Kanniainen wrote:
>> Move led_mode attribute from HID device to led-class devices and rename
>> it msi_mode. This will also fix race condition by using
>
> There's a typo here (s/msi_mode/mode) but perhaps Bryan can just fix
> that up before applying?
>
>> attribute-groups.
>>
>> Signed-off-by: Janne Kanniainen
>
> Reviewed-by: Johan Hovold
>
> Otherwise both patches (v6) are ready to be merged, Bryan.
>
> Thanks, Janne!
>
No problem. I fixed the typo and merged it.
Thanks,
-Bryan
>> ---
>>
>> Changes in v3:
>> - Style fixes
>> - Rename sysfs-class-hid-driver-gt683r to
>> sysfs-class-leds-driver-gt683r
>>
>> Changes in v4:
>> - Rename sysfs-class-leds-driver-gt683r to sysfs-class-leds-gt683r
>> - Change "What: " to /sys/class/leds//gt683r/mode
>> - Change .name from gt683r_led to gt683r
>>
>> Changes in v5:
>> - Move mode attribute to gt683r/mode
>>
>> Changes in v6:
>> - Fix subject and commit message
>>
>> .../ABI/testing/sysfs-class-hid-driver-gt683r | 14 -
>> Documentation/ABI/testing/sysfs-class-leds-gt683r | 16 ++
>> drivers/hid/hid-gt683r.c | 36
>> ++
>> 3 files changed, 40 insertions(+), 26 deletions(-)
>> delete mode 100644 Documentation/ABI/testing/sysfs-class-hid-driver-gt683r
>> create mode 100644 Documentation/ABI/testing/sysfs-class-leds-gt683r
>>
>> diff --git a/Documentation/ABI/testing/sysfs-class-hid-driver-gt683r
>> b/Documentation/ABI/testing/sysfs-class-hid-driver-gt683r
>> deleted file mode 100644
>> index 317e9d5..000
>> --- a/Documentation/ABI/testing/sysfs-class-hid-driver-gt683r
>> +++ /dev/null
>> @@ -1,14 +0,0 @@
>> -What:/sys/class/hidraw//device/leds_mode
>> -Date:Jun 2014
>> -KernelVersion: 3.17
>> -Contact: Janne Kanniainen
>> -Description:
>> - Set the mode of LEDs
>> -
>> - 0 - normal
>> - 1 - audio
>> - 2 - breathing
>> -
>> - Normal: LEDs are fully on when enabled
>> - Audio: LEDs brightness depends on sound level
>> - Breathing: LEDs brightness varies at human breathing rate
>> \ No newline at end of file
>> diff --git a/Documentation/ABI/testing/sysfs-class-leds-gt683r
>> b/Documentation/ABI/testing/sysfs-class-leds-gt683r
>> new file mode 100644
>> index 000..e4fae60
>> --- /dev/null
>> +++ b/Documentation/ABI/testing/sysfs-class-leds-gt683r
>> @@ -0,0 +1,16 @@
>> +What:/sys/class/leds//gt683r/mode
>> +Date:Jun 2014
>> +KernelVersion: 3.17
>> +Contact: Janne Kanniainen
>> +Description:
>> + Set the mode of LEDs. You should notice that changing the mode
>> + of one LED will update the mode of its two sibling devices as
>> + well.
>> +
>> + 0 - normal
>> + 1 - audio
>> + 2 - breathing
>> +
>> + Normal: LEDs are fully on when enabled
>> + Audio: LEDs brightness depends on sound level
>> + Breathing: LEDs brightness varies at human breathing rate
>> \ No newline at end of file
>> diff --git a/drivers/hid/hid-gt683r.c b/drivers/hid/hid-gt683r.c
>> index 073bd80..0d6f135 100644
>> --- a/drivers/hid/hid-gt683r.c
>> +++ b/drivers/hid/hid-gt683r.c
>> @@ -84,12 +84,13 @@ static void gt683r_brightness_set(struct led_classdev
>> *led_cdev,
>> }
>> }
>>
>> -static ssize_t leds_mode_show(struct device *dev,
>> +static ssize_t mode_show(struct device *dev,
>> struct device_attribute *attr,
>> char *buf)
>> {
>> u8 sysfs_mode;
>> - struct hid_device *hdev = container_of(dev, struct hid_device, dev);
>> + struct hid_device *hdev = container_of(dev->parent,
>> + struct hid_device, dev);
>> struct gt683r_led *led = hid_get_drvdata(hdev);
>>
>> if (led->mode == GT683R_LED_NORMAL)
>> @@ -102,12 +103,13 @@ static ssize_t leds_mode_show(struct device *dev,
>> return scnprintf(buf, PAGE_SIZE, "%u\n", sysfs_mode);
>> }
>>
>> -static ssize_t leds_mode_store(struct device *dev,
>> +static ssize_t mode_store(struct device *dev,
>> struct device_attribute *attr,
>> const char *buf, size_t count)
>> {
>> u8 sysfs_mode;
>> - struct hid_device *hdev = container_of(dev, struct hid_device, dev);
>> + struct hid_device *hdev = container_of(dev->parent,
>> + struct hid_device, dev);
>> struct gt683r_led *led = hid_get_drvdata(hdev);
>>
>>
>> @@ -212,7 +214,22 @@ fail:
>> mutex_unlock(&led->lock);
>> }
>>
>> -static DEVICE_ATTR_RW(leds_mode);
>> +static DEVICE_ATTR_RW(mode);
>> +
>> +static struct attribute *gt683r_led_attrs[] = {
>> +
Re: [PATCH 1/2 v6] HID: gt683r: fix race condition
On Thu, Jul 3, 2014 at 10:34 AM, Johan Hovold wrote: > On Thu, Jul 03, 2014 at 08:17:08PM +0300, Janne Kanniainen wrote: >> This will fix race condition noticed by Oliver Neukum. Sysfs files are >> created before mutex and work are initialized. >> >> Signed-off-by: Janne Kanniainen > > Reviewed-by: Johan Hovold > Good, merged to my tree. -Bryan >> --- >> drivers/hid/hid-gt683r.c | 6 +++--- >> 1 file changed, 3 insertions(+), 3 deletions(-) >> >> diff --git a/drivers/hid/hid-gt683r.c b/drivers/hid/hid-gt683r.c >> index 077f7a1..073bd80 100644 >> --- a/drivers/hid/hid-gt683r.c >> +++ b/drivers/hid/hid-gt683r.c >> @@ -227,6 +227,9 @@ static int gt683r_led_probe(struct hid_device *hdev, >> if (!led) >> return -ENOMEM; >> >> + mutex_init(&led->lock); >> + INIT_WORK(&led->work, gt683r_led_work); >> + >> led->mode = GT683R_LED_NORMAL; >> led->hdev = hdev; >> hid_set_drvdata(hdev, led); >> @@ -271,9 +274,6 @@ static int gt683r_led_probe(struct hid_device *hdev, >> goto fail; >> } >> >> - mutex_init(&led->lock); >> - INIT_WORK(&led->work, gt683r_led_work); >> - >> return 0; >> >> fail: -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2 v2] HID: leds: Use attribute-groups in MSI GT683R driver
On Tue, Jul 1, 2014 at 1:48 AM, Johan Hovold wrote:
> On Mon, Jun 30, 2014 at 04:17:10PM -0700, Bryan Wu wrote:
>> On Mon, Jun 30, 2014 at 4:33 AM, Jiri Kosina wrote:
>> > On Mon, 30 Jun 2014, Johan Hovold wrote:
>> >
>> >> > I think the better place is HID/input tree, since this patch depends
>> >> > on the initial one which is not in my tree.
>> >> > I'm going to merge Johan's whole patchset and this patch probably
>> >> > depends Johan's work too.
>> >>
>> >> Dmitry has ACKed the input-patch and Bryan has applied that one and the
>> >> leds-patches to his tree (of which the first one is a dependency of this
>> >> patch).
>> >>
>> >> Jiri, are you saying that the gt683r-driver should go in through his
>> >> tree as well, that is all three patches including the first that you
>> >> have already applied? I just assumed your for-next branch was immutable,
>> >> but perhaps I was mistaken.
>> >
>> > Well, for-next branch is a collection of all the topic branches I am
>> > queuing for the following merge window.
>> >
>> > I am never really rebasing it, but I can definitely not include
>> > 'for-3.17/hid-gt683r' topic branch in the pile I will be sending to Linus
>> > (all the scheduled branches are getting merged into 'for-linus' only when
>> > merge window open). So the only potential conflict between hid.git and
>> > Bryan's tree would be in linux-next (and probably there will be none, git
>> > can handle duplicate patches nicely).
>> >
>> > So once Bryan confirms he's queued it (please preserve my Signoff from my
>> > tree), then I will just not include for-3.17/hid-gt683r branch in pull
>> > request to Linus and all is fine.
>> >
>>
>> I'm OK to merge Janne's first patch for HID GT683R through my tree
>> with you guys' SOB.
>> I'm also OK to merge this incremental patchset here. Please confirm it
>> if I didn't misunderstand here.
>
> That's correct. But the incremental patch set might need one more spin
> before it is ready to be applied.
>
>> Also Janne or someone, can you post the original first patch to me or
>> point me where is it?
>
> You could cherry-pick it from Jiri's for-3.17/hid-gt683r branch at
>
> git://git.kernel.org/pub/scm/linux/kernel/git/jikos/hid.git
>
> commit 4e3ed79670e0 ("HID: add support for MSI GT683R led panels").
>
> Otherwise, the patch is here:
>
> http://marc.info/?l=linux-usb&m=140310755911256&w=2
>
Great, I just cherry-picked and applied to my tree
http://git.kernel.org/cgit/linux/kernel/git/cooloney/linux-leds.git/commit/?h=for-next&id=f471d9480275796dea2ac7ec249b050e70a2888d
Thanks,
-Bryan
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2 v2] HID: leds: Use attribute-groups in MSI GT683R driver
On Mon, Jun 30, 2014 at 4:33 AM, Jiri Kosina wrote: > On Mon, 30 Jun 2014, Johan Hovold wrote: > >> > I think the better place is HID/input tree, since this patch depends >> > on the initial one which is not in my tree. >> > I'm going to merge Johan's whole patchset and this patch probably >> > depends Johan's work too. >> >> Dmitry has ACKed the input-patch and Bryan has applied that one and the >> leds-patches to his tree (of which the first one is a dependency of this >> patch). >> >> Jiri, are you saying that the gt683r-driver should go in through his >> tree as well, that is all three patches including the first that you >> have already applied? I just assumed your for-next branch was immutable, >> but perhaps I was mistaken. > > Well, for-next branch is a collection of all the topic branches I am > queuing for the following merge window. > > I am never really rebasing it, but I can definitely not include > 'for-3.17/hid-gt683r' topic branch in the pile I will be sending to Linus > (all the scheduled branches are getting merged into 'for-linus' only when > merge window open). So the only potential conflict between hid.git and > Bryan's tree would be in linux-next (and probably there will be none, git > can handle duplicate patches nicely). > > So once Bryan confirms he's queued it (please preserve my Signoff from my > tree), then I will just not include for-3.17/hid-gt683r branch in pull > request to Linus and all is fine. > I'm OK to merge Janne's first patch for HID GT683R through my tree with you guys' SOB. I'm also OK to merge this incremental patchset here. Please confirm it if I didn't misunderstand here. Also Janne or someone, can you post the original first patch to me or point me where is it? Thanks, -Bryan -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2 v2] HID: leds: Use attribute-groups in MSI GT683R driver
On Wed, Jun 25, 2014 at 12:09 PM, Jiri Kosina wrote: > On Wed, 25 Jun 2014, Johan Hovold wrote: > >> Did you see the attribute-race series I posted? Not sure how best to >> handle the dependency, as those patches should probably go in through >> the LEDs tree, while the first patch in that series (adding the groups >> field) is a dependency for this patch. >> >> Jiri, how would this best be solved? > > I think the best course of action here is to gather Acks from the > respective maintainers, and take the whole lot trough a single tree > (probably the leds tree in this case) to avoid unnecessary intra-tree > dependencies in a rather straighforward situation like this. > I think the better place is HID/input tree, since this patch depends on the initial one which is not in my tree. I'm going to merge Johan's whole patchset and this patch probably depends Johan's work too. -Bryan -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
