Re: USB deadlock after resume
Hi,
On Wed, 21 Nov 2007 15:37:20 +0100, Markus Rechberger
[EMAIL PROTECTED] wrote:
On 11/21/07, Oliver Neukum [EMAIL PROTECTED] wrote:
Am Mittwoch 21 November 2007 schrieb Markus Rechberger:
On 11/21/07, Markus Rechberger [EMAIL PROTECTED] wrote:
On 11/21/07, Mark Lord [EMAIL PROTECTED] wrote:
Markus Rechberger wrote:
Hi,
I'm looking at the linux uvc driver, and noticed after resuming
my
..
Pardon me.. what is the uvc driver? Which module/source file is
that?
http://linux-uvc.berlios.de/ it's not yet included in the kernel
sources although many distributions already ship it.
A dry run putting the device into sleep mode works fine (I added a
proc interface for calling those suspend/resume function).
it's not just usb_set_interface that hangs actually.
It seems to hang at
wait_event(usb_kill_urb_queue, atomic_read(urb-use_count) == 0);
in drivers/usb/core/urb.c after resuming. I disabled access to the usb
subsystem in the uvc driver, although connecting any other usb storage
fails too, just at the same point.
Which URB is usb_kill_urb() called for?
it's the usb_control_message which calls usb_kill_urb if I haven't got
it wrong. (if you're looking for some other information please let me
know)
Although, I got a bit further with it. The error seems to happen
earlier already.
If I load the driver, and do not access the device after suspending
all usb_control commands fail with -71 eproto.
This device shouldn't be suspended. AFAIK it should have auto_suspend
disabled
by default.
Try adding this device to drivers/usb/core/quirks.c with
USB_QUIRK_NO_AUTOSUSPEND flag set. Something like:
diff --git a/drivers/usb/core/quirks.c b/drivers/usb/core/quirks.c
index d42c561..729eb4b 100644
--- a/drivers/usb/core/quirks.c
+++ b/drivers/usb/core/quirks.c
@@ -45,6 +45,9 @@ static const struct usb_device_id usb_quirk_list[] = {
/* SKYMEDI USB_DRIVE */
{ USB_DEVICE(0x1516, 0x8628), .driver_info = USB_QUIRK_RESET_RESUME
},
+ /* Your device */
+ { USB_DEVICE(0x, 0x), .driver_info =
USB_QUIRK_NO_AUTOSUSPEND },
+
{ } /* terminating entry must be last */
};
Reloading the driver doesn't help at tht point, only reconnecting the
device does.
The data is transfered using bulk transfer.
Markus
-
To unsubscribe from this list: send the line unsubscribe linux-kernel
in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
--
Best Regards,
Felipe Balbi
http://felipebalbi.com
[EMAIL PROTECTED]
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: USB deadlock after resume
On Wed, 21 Nov 2007 15:42:43 +0100, Markus Rechberger [EMAIL PROTECTED] wrote: On 11/21/07, Markus Rechberger [EMAIL PROTECTED] wrote: On 11/21/07, Oliver Neukum [EMAIL PROTECTED] wrote: Am Mittwoch 21 November 2007 schrieb Markus Rechberger: On 11/21/07, Markus Rechberger [EMAIL PROTECTED] wrote: On 11/21/07, Mark Lord [EMAIL PROTECTED] wrote: Markus Rechberger wrote: Hi, I'm looking at the linux uvc driver, and noticed after resuming my .. Pardon me.. what is the uvc driver? Which module/source file is that? http://linux-uvc.berlios.de/ it's not yet included in the kernel sources although many distributions already ship it. A dry run putting the device into sleep mode works fine (I added a proc interface for calling those suspend/resume function). it's not just usb_set_interface that hangs actually. It seems to hang at wait_event(usb_kill_urb_queue, atomic_read(urb-use_count) == 0); in drivers/usb/core/urb.c after resuming. I disabled access to the usb subsystem in the uvc driver, although connecting any other usb storage fails too, just at the same point. Which URB is usb_kill_urb() called for? it's the usb_control_message which calls usb_kill_urb if I haven't got it wrong. (if you're looking for some other information please let me know) Although, I got a bit further with it. The error seems to happen earlier already. If I load the driver, and do not access the device after suspending all usb_control commands fail with -71 eproto. Reloading the driver doesn't help at tht point, only reconnecting the device does. The data is transfered using bulk transfer. Do you know any good way for performing a softreset within the driver? The video application should get a continuous datastream after resuming the notebook, so the driver shouldn't be unloaded. The driver also keeps a list of previous camera settings which should be set up again after resuming. Stopping the video application and reattaching the device using ACPI (this board supports reconnecting the device using ACPI) should be avoided. When you suspend, you cut off vbus (afaik, correct me if I'm wrong), which means your device will get disconnected. One way to avoid this is enabling CONFIG_USB_PERSIST and trying with that on. You should also disable auto_suspend for this device. It won't work ok after that probably. Markus - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/ -- Best Regards, Felipe Balbi http://felipebalbi.com [EMAIL PROTECTED] - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: USB deadlock after resume
On Wed, 21 Nov 2007 15:52:37 +0100, Markus Rechberger [EMAIL PROTECTED] wrote: On 11/21/07, Markus Rechberger [EMAIL PROTECTED] wrote: On 11/21/07, Markus Rechberger [EMAIL PROTECTED] wrote: On 11/21/07, Oliver Neukum [EMAIL PROTECTED] wrote: Am Mittwoch 21 November 2007 schrieb Markus Rechberger: On 11/21/07, Markus Rechberger [EMAIL PROTECTED] wrote: On 11/21/07, Mark Lord [EMAIL PROTECTED] wrote: Markus Rechberger wrote: Hi, I'm looking at the linux uvc driver, and noticed after resuming my .. Pardon me.. what is the uvc driver? Which module/source file is that? http://linux-uvc.berlios.de/ it's not yet included in the kernel sources although many distributions already ship it. A dry run putting the device into sleep mode works fine (I added a proc interface for calling those suspend/resume function). it's not just usb_set_interface that hangs actually. It seems to hang at wait_event(usb_kill_urb_queue, atomic_read(urb-use_count) == 0); in drivers/usb/core/urb.c after resuming. I disabled access to the usb subsystem in the uvc driver, although connecting any other usb storage fails too, just at the same point. Which URB is usb_kill_urb() called for? it's the usb_control_message which calls usb_kill_urb if I haven't got it wrong. (if you're looking for some other information please let me know) Although, I got a bit further with it. The error seems to happen earlier already. If I load the driver, and do not access the device after suspending all usb_control commands fail with -71 eproto. Reloading the driver doesn't help at tht point, only reconnecting the device does. The data is transfered using bulk transfer. Do you know any good way for performing a softreset within the driver? The video application should get a continuous datastream after resuming the notebook, so the driver shouldn't be unloaded. The driver also keeps a list of previous camera settings which should be set up again after resuming. Stopping the video application and reattaching the device using ACPI (this board supports reconnecting the device using ACPI) should be avoided. ok usb_reset_device() did the job after resuming. First problem solved. hmm... in that case try setting in quirks USB_QUIRK_RESET_RESUME Markus - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/ -- Best Regards, Felipe Balbi http://felipebalbi.com [EMAIL PROTECTED] - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [linux-usb-devel] [PATCH] : Allow embedded developers USB options normally reserved for OTG
On Wed, Jan 02, 2008 at 10:47:15AM -0800, David Brownell wrote:
On Wednesday 02 January 2008, Robin Getz wrote:
From: Robin Getz [EMAIL PROTECTED]
Allow embedded developers to turn support for USB Hubs off even if they
have a
full root hub. This saves the overhead (RAM and Flash size).
ISTR that it won't save very much code though ... the Linux USB
stack structures all its enumeration logic around hubs.
Allow embedded developers the capabilities of the otg_whitelist.h - a
product whitelist, so USB peripherals not listed there will be rejected
during enumeration. This is the desired operation for many embedded
products.
Signed-off-by: Robin Getz [EMAIL PROTECTED]
This is probably the right thing to do. Correct me if I'm wrong,
but USB-IF recently put out some specs about embedded hosts which
basically boil down to saying you can adopt the same functionality
restrictions that used to be OTG-only. Which is why now there are
embedded developers who'd like this option. :)
otg whitelist is not a blocker. A device that is not listed on TPL
might or might not work. The logic around that should be something like:
parse_whitelist()
if (!listed) {
match_class_with_tpled_devices();
if (match_any)
check_power_need();
if (power_need = 100mA)
allow_enumeration();
else
deny_enumaration_and_message();
If you check n810, you'll see that even though a usb memory stick is not
listed on its TPL, we allow it to enumerate as long as it draws less
then 100mA and this is true on something around 95% of usb memory
sticks.
I'm kinda busy with other tasks right now, but when I finish formating
proper patches for current mainline head, I'll release some code adding
support for dynamic otg tpl. The problem I see here is that my only
user is musb driver, currently only available on linux-omap git tree.
Maybe it's time to send it to mainline, what do you think Dave?
In any case, I'll format such patches when I'm done with twl4030 and
isp1301 development, something around 3 ~ 4 weeks from now.
Even though, embedded hosts should have the same behavior.
Best Regards,
Felipe Balbi
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH 0/2] ISP1301 updates
Hello all, The following two patches updates isp1301_omap.c to new-style i2c driver. The patches are against curent linux mailine head. Tony Lindgren (Cc'ed here) already has a working version of this driver on his linux-omap git tree (see [1] and [2]), these are just a rework for them to apply cleanly on mainline. The patches were compile tested with omap_h2_1610_defconfig with isp1301 enabled. [1] http://git.kernel.org/?p=linux/kernel/git/tmlind/linux-omap-2.6.git;a=commit;h=d8e0e848e562deaa405a2014240d6deedf3bfd37 [2] http://git.kernel.org/?p=linux/kernel/git/tmlind/linux-omap-2.6.git;a=commit;h=d24c8950a363044016679cdf76435abdace2f56d BR, Felipe Balbi -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 1/2] I2C: ISP1301_OMAP: New-style i2c driver updates, part 1
Based on David Brownell's patch for tps65010, this patch
starts converting isp1301_omap.c to new-style i2c driver.
Signed-off-by: Felipe Balbi [EMAIL PROTECTED]
---
drivers/i2c/chips/isp1301_omap.c | 60 +++--
1 files changed, 31 insertions(+), 29 deletions(-)
diff --git a/drivers/i2c/chips/isp1301_omap.c b/drivers/i2c/chips/isp1301_omap.c
index b767603..37e1403 100644
--- a/drivers/i2c/chips/isp1301_omap.c
+++ b/drivers/i2c/chips/isp1301_omap.c
@@ -49,7 +49,8 @@ MODULE_LICENSE(GPL);
struct isp1301 {
struct otg_transceiver otg;
- struct i2c_client client;
+ struct i2c_client *client;
+ struct i2c_client c;
void(*i2c_release)(struct device *dev);
int irq;
@@ -153,25 +154,25 @@ static struct i2c_driver isp1301_driver;
static inline u8
isp1301_get_u8(struct isp1301 *isp, u8 reg)
{
- return i2c_smbus_read_byte_data(isp-client, reg + 0);
+ return i2c_smbus_read_byte_data(isp-client, reg + 0);
}
static inline int
isp1301_get_u16(struct isp1301 *isp, u8 reg)
{
- return i2c_smbus_read_word_data(isp-client, reg);
+ return i2c_smbus_read_word_data(isp-client, reg);
}
static inline int
isp1301_set_bits(struct isp1301 *isp, u8 reg, u8 bits)
{
- return i2c_smbus_write_byte_data(isp-client, reg + 0, bits);
+ return i2c_smbus_write_byte_data(isp-client, reg + 0, bits);
}
static inline int
isp1301_clear_bits(struct isp1301 *isp, u8 reg, u8 bits)
{
- return i2c_smbus_write_byte_data(isp-client, reg + 1, bits);
+ return i2c_smbus_write_byte_data(isp-client, reg + 1, bits);
}
/*-*/
@@ -355,10 +356,10 @@ isp1301_defer_work(struct isp1301 *isp, int work)
int status;
if (isp !test_and_set_bit(work, isp-todo)) {
- (void) get_device(isp-client.dev);
+ (void) get_device(isp-client-dev);
status = schedule_work(isp-work);
if (!status !isp-working)
- dev_vdbg(isp-client.dev,
+ dev_vdbg(isp-client-dev,
work item %d may be lost\n, work);
}
}
@@ -1113,7 +1114,7 @@ isp1301_work(struct work_struct *work)
/* transfer state from otg engine to isp1301 */
if (test_and_clear_bit(WORK_UPDATE_ISP, isp-todo)) {
otg_update_isp(isp);
- put_device(isp-client.dev);
+ put_device(isp-client-dev);
}
#endif
/* transfer state from isp1301 to otg engine */
@@ -1121,7 +1122,7 @@ isp1301_work(struct work_struct *work)
u8 stat = isp1301_clear_latch(isp);
isp_update_otg(isp, stat);
- put_device(isp-client.dev);
+ put_device(isp-client-dev);
}
if (test_and_clear_bit(WORK_HOST_RESUME, isp-todo)) {
@@ -1156,7 +1157,7 @@ isp1301_work(struct work_struct *work)
}
host_resume(isp);
// mdelay(10);
- put_device(isp-client.dev);
+ put_device(isp-client-dev);
}
if (test_and_clear_bit(WORK_TIMER, isp-todo)) {
@@ -1165,15 +1166,15 @@ isp1301_work(struct work_struct *work)
if (!stop)
mod_timer(isp-timer, jiffies + TIMER_JIFFIES);
#endif
- put_device(isp-client.dev);
+ put_device(isp-client-dev);
}
if (isp-todo)
- dev_vdbg(isp-client.dev,
+ dev_vdbg(isp-client-dev,
work done, todo = 0x%lx\n,
isp-todo);
if (stop) {
- dev_dbg(isp-client.dev, stop\n);
+ dev_dbg(isp-client-dev, stop\n);
break;
}
} while (isp-todo);
@@ -1197,7 +1198,7 @@ static void isp1301_release(struct device *dev)
{
struct isp1301 *isp;
- isp = container_of(dev, struct isp1301, client.dev);
+ isp = container_of(dev, struct isp1301, c.dev);
/* ugly -- i2c hijacks our memory hook to wait_for_completion() */
if (isp-i2c_release)
@@ -1211,7 +1212,7 @@ static int isp1301_detach_client(struct i2c_client *i2c)
{
struct isp1301 *isp;
- isp = container_of(i2c, struct isp1301, client);
+ isp = container_of(i2c, struct isp1301, c);
isp1301_clear_bits(isp, ISP1301_INTERRUPT_FALLING, ~0);
isp1301_clear_bits(isp, ISP1301_INTERRUPT_RISING, ~0);
@@ -1263,7 +1264,7 @@ static int isp1301_otg_enable(struct isp1301 *isp
[PATCH 2/2] I2C: ISP1301_OMAP: New-style i2c driver updates, part 2
Based on David Brownell's patch for tps65010, this patch
finish conversting isp1301_omap.c to new-style i2c driver.
Signed-off-by: Felipe Balbi [EMAIL PROTECTED]
---
arch/arm/mach-omap1/board-h2.c |6 ++-
arch/arm/mach-omap1/board-h3.c |6 ++-
arch/arm/mach-omap2/board-h4.c | 12 +++
drivers/i2c/chips/isp1301_omap.c | 149 +-
4 files changed, 73 insertions(+), 100 deletions(-)
diff --git a/arch/arm/mach-omap1/board-h2.c b/arch/arm/mach-omap1/board-h2.c
index 1306812..0307f50 100644
--- a/arch/arm/mach-omap1/board-h2.c
+++ b/arch/arm/mach-omap1/board-h2.c
@@ -350,8 +350,12 @@ static struct i2c_board_info __initdata
h2_i2c_board_info[] = {
.type = tps65010,
.irq= OMAP_GPIO_IRQ(58),
},
+ {
+ I2C_BOARD_INFO(isp1301_omap, 0x2d),
+ .type = isp1301_omap,
+ .irq= OMAP_GPIO_IRQ(2),
+ },
/* TODO when driver support is ready:
-* - isp1301 OTG transceiver
* - optional ov9640 camera sensor at 0x30
* - pcf9754 for aGPS control
* - ... etc
diff --git a/arch/arm/mach-omap1/board-h3.c b/arch/arm/mach-omap1/board-h3.c
index 4f84ae2..71e285a 100644
--- a/arch/arm/mach-omap1/board-h3.c
+++ b/arch/arm/mach-omap1/board-h3.c
@@ -463,8 +463,12 @@ static struct i2c_board_info __initdata
h3_i2c_board_info[] = {
.type = tps65013,
/* .irq = OMAP_GPIO_IRQ(??), */
},
+ {
+ I2C_BOARD_INFO(isp1301_omap, 0x2d),
+ .type = isp1301_omap,
+ .irq= OMAP_GPIO_IRQ(14),
+ },
/* TODO when driver support is ready:
-* - isp1301 OTG transceiver
* - optional ov9640 camera sensor at 0x30
* - ...
*/
diff --git a/arch/arm/mach-omap2/board-h4.c b/arch/arm/mach-omap2/board-h4.c
index f125f43..33ff80b 100644
--- a/arch/arm/mach-omap2/board-h4.c
+++ b/arch/arm/mach-omap2/board-h4.c
@@ -301,6 +301,15 @@ static struct omap_board_config_kernel h4_config[] = {
{ OMAP_TAG_LCD, h4_lcd_config },
};
+static struct i2c_board_info __initdata h4_i2c_board_info[] = {
+ {
+ I2C_BOARD_INFO(isp1301_omap, 0x2d),
+ .type = isp1301_omap,
+ .irq= OMAP_GPIO_IRQ(125),
+ },
+};
+
+
static void __init omap_h4_init(void)
{
/*
@@ -321,6 +330,9 @@ static void __init omap_h4_init(void)
}
#endif
+ i2c_register_board_info(1, h4_i2c_board_info,
+ ARRAY_SIZE(h4_i2c_board_info));
+
platform_add_devices(h4_devices, ARRAY_SIZE(h4_devices));
omap_board_config = h4_config;
omap_board_config_size = ARRAY_SIZE(h4_config);
diff --git a/drivers/i2c/chips/isp1301_omap.c b/drivers/i2c/chips/isp1301_omap.c
index 37e1403..c7a7c48 100644
--- a/drivers/i2c/chips/isp1301_omap.c
+++ b/drivers/i2c/chips/isp1301_omap.c
@@ -31,16 +31,12 @@
#include linux/usb/otg.h
#include linux/i2c.h
#include linux/workqueue.h
-
-#include asm/irq.h
#include asm/arch/usb.h
-
#ifndefDEBUG
#undef VERBOSE
#endif
-
#defineDRIVER_VERSION 24 August 2004
#defineDRIVER_NAME (isp1301_driver.driver.name)
@@ -50,12 +46,8 @@ MODULE_LICENSE(GPL);
struct isp1301 {
struct otg_transceiver otg;
struct i2c_client *client;
- struct i2c_client c;
void(*i2c_release)(struct device *dev);
- int irq;
- int irq_type;
-
u32 last_otg_ctrl;
unsignedworking:1;
@@ -140,13 +132,6 @@ static inline void notresponding(struct isp1301 *isp)
/*-*/
/* only two addresses possible */
-#defineISP_BASE0x2c
-static unsigned short normal_i2c[] = {
- ISP_BASE, ISP_BASE + 1,
- I2C_CLIENT_END };
-
-I2C_CLIENT_INSMOD;
-
static struct i2c_driver isp1301_driver;
/* smbus apis are used for portability */
@@ -1196,9 +1181,11 @@ static void isp1301_timer(unsigned long _isp)
static void isp1301_release(struct device *dev)
{
- struct isp1301 *isp;
+ struct i2c_client *client;
+ struct isp1301 *isp;
- isp = container_of(dev, struct isp1301, c.dev);
+ client = container_of(dev, struct i2c_client, dev);
+ isp = i2c_get_clientdata(client);
/* ugly -- i2c hijacks our memory hook to wait_for_completion() */
if (isp-i2c_release)
@@ -1208,30 +1195,27 @@ static void isp1301_release(struct device *dev)
static struct isp1301 *the_transceiver;
-static int isp1301_detach_client(struct i2c_client *i2c)
+static int __exit isp1301_remove(struct i2c_client *client)
{
- struct isp1301 *isp;
-
- isp
[PATCH] USB: OTG: Fix weirdnesses on enumerating partial otg devices
Remove the check for is_b_host upon enumerating otg devices as it
can trigger some weird behaviors on otg sessions. Some devices claim
to be b_host even though they have an a_connector attached to it.
Checking b_hnp_enable flag should be secure enough or terms of
otg compliancy.
Signed-off-by: Felipe Balbi [EMAIL PROTECTED]
---
drivers/usb/core/hub.c |2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index 68fc521..963cf9b 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -1315,7 +1315,7 @@ static int usb_configure_device_otg(struct usb_device
*udev)
/* Maybe it can talk to us, though we can't talk to it.
* (Includes HNP test device.)
*/
- if (udev-bus-b_hnp_enable || udev-bus-is_b_host) {
+ if (udev-bus-b_hnp_enable) {
err = usb_port_suspend(udev);
if (err 0)
dev_dbg(udev-dev, HNP fail, %d\n, err);
--
1.5.4.34.g053d9
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] USB: OTG: Fix weirdnesses on enumerating partial otg devices
On Tue, Feb 12, 2008 at 05:02:47AM -0800, David Brownell wrote: On Tuesday 12 February 2008, Felipe Balbi wrote: On Tue, Feb 12, 2008 at 02:28:53AM -0800, David Brownell wrote: On Tuesday 12 February 2008, Felipe Balbi wrote: Some devices claim to be b_host even though they have an a_connector attached to it. Why not just fix that bug? Remember that's Linux code. The device claiming to be b_host is not linux based. Wrong ... the meaning of that flag is: *THIS* system is a Linux-USB OTG device which came up in B-peripheral role, and then through the magic of HNP morphed into the B-host role. Don't be sarcastic I won't even fall into what Jean Delvare told you. Linux is acting as a host at that point. So either it's being the A-host, or the B-host. That flag says which. If the other device has the A-connector, yet we're the B-host, then right now it is acting as an A-peripheral. That's exactly what HNP is designed to do. True, but allowing hnp nowing we're b_host doesn't sound correct. If we're b_host it can get two scenarios we would get to fall into this state: 1. We asked to become host; or 2. A_device allowed us become host because it can't handle us. In both cases, HNP is quite useless happening again, don't you think? We should at least try to enumerate a_peripheral, if it can't due to our power capabilities i.e. 100mA, we'll fall into overcurrent protection and we'll suspend the bus and disconnect, which would give a_device another change to enumerate us. This sound much better to me. In any case this is_b_host check won't do nothing here as we should check is SetFeature(b_hnp_enbable) has been succesfull. Again wrong ... if the host side's b_hnp_enable flag is set, that means this is a Linux-USB OTG device which came up in A-host role and enumerated some vendor's OTG device, and then set the b_hnp_enable flag. (So it could in the future use HNP to become an A-peripheral, despite having started as an A-host.) A device in b_host state is not enough for allowing hnp to happen. If the system is in b_host state, then HNP *ALREADY* happened. True, but... So it can happen again, to go back into the b_peripheral state. (And of course, we can't set the b_hnp_enable flag in a device which is in the a_peripheral state...) it shouldn't happen again at this point. If we're b_host, we should try to enumerate a_peripheral. If it draws more than 100mA the session should end, otherwise we'd fall into hnp loops until we start getting hub port errors and linux decide to suspend roothub and end the session. Seems like the root cause of the problem here is that you have some correctly functioning code, but for some reason you're surprised by that correct functioning. (Maybe this is a case where neither device is on the other's whitelist?) True, neither is in other's whitelist, but the point is: 1. Currently standard-a device supports hnp only on A states 2. Currently standard-b device supports hnp on both roles 3. A-device enumerates b-device, checks it's not tpl'ed and allow hnp 4. B-device become b_host, checks a_peripheral is not on its tpl, even though it has support for that device class and it draws less than 100mA. According to otg tpl clarification, they should work in this scenario, shouldn't they ? Or should they stay in an hnp loop until someone decides to end the session ? -- - Balbi -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] USB: OTG: Fix weirdnesses on enumerating partial otg devices
On Tue, Feb 12, 2008 at 02:28:53AM -0800, David Brownell wrote: On Tuesday 12 February 2008, Felipe Balbi wrote: Some devices claim to be b_host even though they have an a_connector attached to it. Why not just fix that bug? Remember that's Linux code. The device claiming to be b_host is not linux based. In any case this is_b_host check won't do nothing here as we should check is SetFeature(b_hnp_enbable) has been succesfull. A device in b_host state is not enough for allowing hnp to happen. Remember some devices doesn't support hnp in B roles, only a_hnp_support is mandatory. -- - Balbi -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] USB: OTG: Fix weirdnesses on enumerating partial otg devices
On Wed, Feb 13, 2008 at 01:36:00PM -0800, David Brownell wrote: On Wednesday 13 February 2008, Felipe Balbi wrote: On Tue, Feb 12, 2008 at 12:32:34PM -0800, David Brownell wrote: Your proposal is to strike the is_b_host check. In terms of the OTG (1.3) state machine, that removes a b_host -- b_peripheral state transition. Not at all, we're just not doing transition right now. When *would* it happen then? And why not do it as soon as it's known that the device on the other end of the cable is unsupported? Whenever we finnish using the bus we'd suspend it. This would sinalize a_peripheral switch back to a_host. Also, even though it's not on our TPL it doesn't mean we can't work with it. BUT: notice it doesn't replace it with the ONLY other valid state transition, b_host -- b_idle. In fact, that can not be initiated by the B-device... So the current code *is* correct. ... deletia ... We should at least try to enumerate a_peripheral, if it can't due to our power capabilities i.e. 100mA, we'll fall into overcurrent protection and we'll suspend the bus and disconnect, which would give a_device another change to enumerate us. This sound much better to me. You're overlooking something: this is the !is_targeted(udev) case, so we have already enumerated the a_peripheral as much as we can. There is *nothing* more we can do with it. Sitting around in the b_host state would be utterly pointless. And you're overlooking something: tpl is not an enumeration blocker at all. The only blockers are drivers and power limits. The device has already been enumerated at this point. This is just whether to set up communication with, and use, the device ... which activity *IS* predicated on device support, on the TPL (as it says in 6.8.1.4 of the spec for the A_HOST role, later reusing that language where it describes the B_HOST role). It only tell us to output a message to the user indicating it's not supported. Same for b_host. It never says we should end the session at that point. OTG Rev 1.3 is unclear about this. That's why I consulted Richard Pietre and asked him what would be the most sane way of handling hnp and he told me to implement it always trying to work with the device even though it's not tpl'ed. Imagine what happens on n810, we have 3 mass storage devices listed on its tpl: 770, n800 and itself. Besides that, we allow enumeration of any mass storage device as long as it draws less than 100mA. While it makes sense to me that an OTG device support things like mass storage class devices, I still see that spec language saying that classes can't be on the TPL ... and that only devices on the TPL get communication beyond what's needed for enumeration. It doesn't say your last point. Besides, it says: The Targeted Peripheral List shall not list supported USB Classes or similar devices. By similar we can understand several mass storage devices. It doesn't really matter on usb otg point of view whether we support mass storage device A or mass storage device B, both should work if they meet otg requirements (power consumption being the most significant issue). We can also see the TPL example on otg rev 1.3 is not listing similar devices. They are all different. Quoting from otg rev 1.3 Section 3.3: Vendor Model Speed TransportVIDPID Description 1. Logitech M-BJ58LS Int 0x046D 0xC00E USB Wheel Mouse 2. Yamaha YST-MS35D FS Isoch 0x0499 0x3002 USB Speakers 3. TEAC Corporation FD-05PUB FS Bulk 0x0644 0x USB Floppy Drive 4. Hewlett Packard D125XIFS Bulk 0x03F0 0x2311 All-In-One Printer/Scanner/Copier This sounds like another clue that we should be able to try to work with any mouse, any speekers, any floppy and any printer as long as they meet power requirements because we have support for them built into the otg device. Regardless, that's a red herring. If you allow sane things like arbitrary flash memory sticks to be used, you've effectively put a device class on the TPL. But this discussion is about things that are NOT on the TPL. No cuz I'm still notifying my user the device is unsupported even though it's not even a requirement, if the device meet otg constraints*(explained below). Which means it might or might not work. If it happen to work, that's fine, if it doesn't the user has been notified about that possibility. One other thing Device is Not Supported is just an example message. * In section 3.4 OTG says: (...) and the OTG device does not support the type of communitcation being requested by the user, then the OTG device shall provide messages to the user that allow him or her to understand the problem (...) Which means that we should notify our user if, and only if, we
Re: [PATCHv2] Input: omap4-keypad: Add pinctrl support
Hi, On Tue, Oct 30, 2012 at 11:24:10AM +, Mark Brown wrote: This is why I think hiding things from drivers makes no sense. Also consider the situations Linus W exposed on another subthread. If you change ordering of certain calls, you will really break the functionality of the IP. Because we can't make sure this won't work automagically in all cases (just like we can't make sure $size memory allocation is enough for all drivers) we don't hide that from the driver. We require driver to manage its resources properly. We need some place to put the SoC integration; power domains seem like the obvious place to me but YMMV. Nothing about having this out of the except that pin muxing has nothing to do with power domain. To me that sounds like an abuse of the API. drivers requires that this be done by individual subsystems in isolation from each other. Half the point here is that for the reusable IPs this stuff often isn't driver specific at all, it's often more about the SoC integration than it is about the driver and so you'll get a consistent pattern for most IPs on the SoC. and all of that SoC-specific detail is already hidden behind power domains, runtime PM, pinctrl, clk API, regulator framework, etc. How can you make sure that this will work for at least 50% of the drivers ? You just can't. We don't know the implementation details of every arch/soc/platform supported by Linux today to make that decision. Well, we've managed to get along for rather a long time with essentially all architectures implementing this stuff by doing static setup for the pins on boot. That does suggest we can get a reasonably long way with and this is one of the issues we're all trying to solve today so we have single zImage approach for the ARM port. something simple, and it does seem to match up with how things usually look at an electrical level too. simple ? I really doubt it. Just look at the amount of code duplication the ARM port had (still has in some places) to handle platform-specific details. It turned out that drivers weren't very portable when they had to do platform-specific initialization, we were all abusing platform_data to pass strings and/or function pointers down to drivers and so on. I'm concerned if we hide pinctrl under e.g. power domains (as said above, it sounds like an abuse of the API to me) we will end up with a situation like above. Maybe not as bad, but still weird-looking. It seems fairly obvious that if we need to add identical bolier plate code to lots of drivers we're doing something wrong, it's just churn for little practical gain and a problem if we ever decide to change how this stuff works in the future. I wouldn't consider it boilerplate if you remember that each driver might have different requirements regarding how all of those details need to be handled. We have to consider power consumption, ordering of calls, proper IP setup, IP reuse on multiple platforms (even multiple ARCHes), etc etc, and to get that right outside of the driver - who's the only class that actually knows what it needs to do with its resources - will just be too complex and error-prone. I would strongly suggest starting with explicit calls to pinctrl, clk API, etc and if we can really prove later that 95% of the users are standard, then we can factor code out, but making that assumption now is quite risky IMHO. -- balbi signature.asc Description: Digital signature
Re: [PATCH 2/3] dma: move dw_dmac driver to an own directory
Hi,
On Tue, Oct 30, 2012 at 02:32:11PM +0200, Andy Shevchenko wrote:
The dw_dmac driver contains multiple files. To make a management of them more
convenient move it to an own directory.
Signed-off-by: Andy Shevchenko andriy.shevche...@linux.intel.com
Reviewed-by: Felipe Balbi ba...@ti.com
Acked-by: Viresh Kumar viresh.ku...@linaro.org
---
drivers/dma/Makefile|3 +--
drivers/dma/dw/Makefile |2 ++
drivers/dma/{ = dw}/dw_dmac.c |2 +-
drivers/dma/{ = dw}/dw_dmac_pci.c |0
drivers/dma/{ = dw}/dw_dmac_regs.h |0
5 files changed, 4 insertions(+), 3 deletions(-)
create mode 100644 drivers/dma/dw/Makefile
rename drivers/dma/{ = dw}/dw_dmac.c (99%)
rename drivers/dma/{ = dw}/dw_dmac_pci.c (100%)
rename drivers/dma/{ = dw}/dw_dmac_regs.h (100%)
diff --git a/drivers/dma/Makefile b/drivers/dma/Makefile
index 15eef5f..122a48a 100644
--- a/drivers/dma/Makefile
+++ b/drivers/dma/Makefile
@@ -11,8 +11,7 @@ obj-$(CONFIG_INTEL_IOP_ADMA) += iop-adma.o
obj-$(CONFIG_FSL_DMA) += fsldma.o
obj-$(CONFIG_MPC512X_DMA) += mpc512x_dma.o
obj-$(CONFIG_MV_XOR) += mv_xor.o
-obj-$(CONFIG_DW_DMAC) += dw_dmac.o
-obj-$(CONFIG_DW_DMAC_PCI) += dw_dmac_pci.o
+obj-$(CONFIG_DW_DMAC) += dw/
this is wrong, I guess. If you want dw_dmac_pci.o to be statically
built-in, but dw_dmac.o to be a dynamic module, according to this
Makefile entry, dw_dmac_pci.o will be linked to vmlinux.
--
balbi
signature.asc
Description: Digital signature
Re: [PATCHv2] Input: omap4-keypad: Add pinctrl support
Hi, On Tue, Oct 30, 2012 at 02:07:15PM +, Mark Brown wrote: On Tue, Oct 30, 2012 at 01:49:49PM +0200, Felipe Balbi wrote: On Tue, Oct 30, 2012 at 11:24:10AM +, Mark Brown wrote: We need some place to put the SoC integration; power domains seem like the obvious place to me but YMMV. Nothing about having this out of the except that pin muxing has nothing to do with power domain. To me that sounds like an abuse of the API. Well, we can call the API Archibald if we like... what I mean is I'm sure you know it's not at all about the name and much more about the semantics ;-) something that sits in the system below the device in pretty much exactly the way that power domains do. drivers requires that this be done by individual subsystems in isolation from each other. Half the point here is that for the reusable IPs this stuff often isn't driver specific at all, it's often more about the SoC integration than it is about the driver and so you'll get a consistent pattern for most IPs on the SoC. and all of that SoC-specific detail is already hidden behind power domains, runtime PM, pinctrl, clk API, regulator framework, etc. That's all getting rather open coded, especially if you're going to get down to a situation where you have varying ordering constraints between the different APIs on different SoCs. however we need to consider those cases right ? Otherwise we will endup pushing something to mainline which will have to be reverted couple weeks later after a big rant from Linus ;-) How can you make sure that this will work for at least 50% of the drivers ? You just can't. We don't know the implementation details of every arch/soc/platform supported by Linux today to make that decision. Well, we've managed to get along for rather a long time with essentially all architectures implementing this stuff by doing static setup for the pins on boot. That does suggest we can get a reasonably long way with and this is one of the issues we're all trying to solve today so we have single zImage approach for the ARM port. I don't see the relevance of single zImage here; device tree is supposed to solve that one. DT is part of the deal. DT alone will solve nothing. something simple, and it does seem to match up with how things usually look at an electrical level too. simple ? I really doubt it. Just look at the amount of code duplication the ARM port had (still has in some places) to handle platform-specific details. What I'm saying here is that I'm concerned about us ending up with more code duplication... a fair concern. It turned out that drivers weren't very portable when they had to do platform-specific initialization, we were all abusing platform_data to pass strings and/or function pointers down to drivers and so on. I'm concerned if we hide pinctrl under e.g. power domains (as said above, it sounds like an abuse of the API to me) we will end up with a situation like above. Maybe not as bad, but still weird-looking. Well, nothing's going to stop that happening if people are determined enough - one could equally see that there'll be flags getting passed to control the ordering of calls if things are open coded. I would expect that with a power domain style approach any data would be being passed directly and bypassing the driver completely. situations like that would be a lot more rare in open coded case, don't you think ? Also a lot more local, since they will show up on a driver source code which is used in a small set of use cases, instead of being part of the pm domain implementation for the entire platform. It seems fairly obvious that if we need to add identical bolier plate code to lots of drivers we're doing something wrong, it's just churn for little practical gain and a problem if we ever decide to change how this stuff works in the future. I wouldn't consider it boilerplate if you remember that each driver might have different requirements regarding how all of those details need to be handled. Essentially all the patches I'm seeing adding pinctrl are totally mindless, most of them are just doing the initial setup on boot and not doing any active management at runtime at all. have you considered that might be just a first step ? I have mentioned this before. We first add the bare minimum and work on PM optimizations later. You can be sure most likely those mindless patches you're seeing are probably building blocks for upcoming patches adding sleep/idle modes. We have to consider power consumption, ordering of calls, proper IP setup, IP reuse on multiple platforms (even multiple ARCHes), etc etc, and to get that right outside of the driver - who's the only class that actually knows what it needs to do with its resources - will just be too complex and error-prone. A big part of my point here is that it's not at all clear to me
Re: [PATCHv2] Input: omap4-keypad: Add pinctrl support
Hi, On Tue, Oct 30, 2012 at 03:58:21PM +, Mark Brown wrote: and this is one of the issues we're all trying to solve today so we have single zImage approach for the ARM port. I don't see the relevance of single zImage here; device tree is supposed to solve that one. DT is part of the deal. DT alone will solve nothing. If DT isn't relevant I'm not sure what you're saying about single I didn't say DT is irrelevant. I said it's not the *only* thing. zImage? The only relevance I can see for that is the data and configuration bloating the kernel, something that DT is supposed to address - this is the main use case where DT has benefits. Well, nothing's going to stop that happening if people are determined enough - one could equally see that there'll be flags getting passed to control the ordering of calls if things are open coded. I would expect that with a power domain style approach any data would be being passed directly and bypassing the driver completely. situations like that would be a lot more rare in open coded case, don't you think ? Also a lot more local, since they will show up on a driver source code which is used in a small set of use cases, instead of being part of the pm domain implementation for the entire platform. I don't see how open coding helps prevent people doing silly things, it seems like it'd have at most neutral impact (and of course it does require going round all the drivers every time someone comes up with a new idea for doing things which is a bit tedious). Essentially all the patches I'm seeing adding pinctrl are totally mindless, most of them are just doing the initial setup on boot and not doing any active management at runtime at all. have you considered that might be just a first step ? I have mentioned this before. We first add the bare minimum and work on PM optimizations later. You can be sure most likely those mindless patches you're seeing are probably building blocks for upcoming patches adding sleep/idle modes. The sleep/idle modes are just a basic extension of the same idea, I'd not anticipate much difference there (and indeed anything where pinmux power saving makes a meaningful impact will I suspect need to be using runtime PM to allow SoC wide power savings anyway). A big part of my point here is that it's not at all clear to me that it is the driver which knows what's going on. For SoC-specific IPs you can be confident about how the SoC integration looks but for IPs that get reused widely this becomes less true. I don't think so. As long as we keep the meaning of the 'default' pinctrl state to mean that this is the working state for the IP, underlying pinctrl-$arch implementation should know how to set muxing up properly, no ? But then this comes round to the mindless code that ought to be factored out :) Only the more interesting cases that do something unusual really register here. fair enough. I see your point. Not saying I agree though, just that this discussion has been flying for far too long, so feel free to provide patches implementing what you're defending here ;-) Guess code will speak for itself. On way or another, we need OMAP keypad driver working in mainline and I don't think your arguments are strong enough to keep $SUBJECT from being merged, unless you can provide something stable/tested for v3.8 merge window. -- balbi signature.asc Description: Digital signature
Re: [PATCHv2] Input: omap4-keypad: Add pinctrl support
Hi, On Tue, Oct 30, 2012 at 11:20:07AM -0700, Dmitry Torokhov wrote: On Tue, Oct 30, 2012 at 07:25:13PM +0200, Felipe Balbi wrote: On Tue, Oct 30, 2012 at 03:58:21PM +, Mark Brown wrote: But then this comes round to the mindless code that ought to be factored out :) Only the more interesting cases that do something unusual really register here. fair enough. I see your point. Not saying I agree though, just that this discussion has been flying for far too long, so feel free to provide patches implementing what you're defending here ;-) Guess code will speak for itself. On way or another, we need OMAP keypad driver working in mainline Are you saying that introducing pincrtl infrastructure actually _broke_ the driver in mainline? no dude, I'm saying we need pinctrl working for this driver because we need to remove OMAP-specific MUX settings. One way or another, this has to work. -- balbi signature.asc Description: Digital signature
Re: [PATCH] i2c: pinctrl-ify i2c-omap.c
Hi, On Wed, Oct 31, 2012 at 05:55:30PM +0200, Pantelis Antoniou wrote: Enable pinctrl for i2c-omap. Signed-off-by: Pantelis Antoniou pa...@antoniou-consulting.com --- drivers/i2c/busses/i2c-omap.c | 10 ++ 1 file changed, 10 insertions(+) diff --git a/drivers/i2c/busses/i2c-omap.c b/drivers/i2c/busses/i2c-omap.c index db31eae..4c38aa0 100644 --- a/drivers/i2c/busses/i2c-omap.c +++ b/drivers/i2c/busses/i2c-omap.c @@ -44,6 +44,8 @@ #include linux/i2c-omap.h #include linux/pm_runtime.h #include linux/pm_qos.h +#include linux/pinctrl/consumer.h +#include linux/err.h /* I2C controller revisions */ #define OMAP_I2C_OMAP1_REV_2 0x20 @@ -1064,6 +1066,7 @@ omap_i2c_probe(struct platform_device *pdev) const struct of_device_id *match; int irq; int r; + struct pinctrl *pinctrl; /* NOTE: driver uses the static register mapping */ mem = platform_get_resource(pdev, IORESOURCE_MEM, 0); @@ -1202,6 +1205,13 @@ omap_i2c_probe(struct platform_device *pdev) of_i2c_register_devices(adap); + pinctrl = devm_pinctrl_get_select_default(pdev-dev); + if (IS_ERR(pinctrl)) + dev_warn(dev-dev, unable to select pin group\n); if we continue anyway, should this be dev_warn() ? Would you consider dev_dbg() instead ? + dev_info(dev-dev, bus %d rev%d.%d.%d at %d kHz\n, adap-nr, + dev-dtrev, dev-rev 4, dev-rev 0xf, dev-speed); this dev_info() doesn't look like it's part of $subject. Care to, at least, add a reasoning for adding this to commit log ? -- balbi signature.asc Description: Digital signature
Re: [PATCH] usb/gadget: let file storage gadget select libcomposite
Hi, On Wed, Oct 24, 2012 at 08:47:04PM +0200, Michal Nazarewicz wrote: On Wed, Oct 24, 2012 at 07:30:42PM +0200, Michal Nazarewicz wrote: At first it looks strange as FSG does not use composite, but yeah: On Wed, Oct 24 2012, Sebastian Andrzej Siewior wrote: Yeah. However, it should be removed in v3.8 anyway :) Yep, that's what feature-removal-schedule.txt says. :] except that it was dropped by commit 9c0ece069b32e8e122aea71aa47181c10eb85ba7 nevertheless, I should be receiving a patch right about now removing that file. Alan, care to send a patch for that ? -- balbi signature.asc Description: Digital signature
Re: [PATCH] [trivial] usb: Fix typo in drivers/usb
Hi, On Tue, Oct 30, 2012 at 12:30:59PM -0700, Greg KH wrote: On Fri, Oct 26, 2012 at 11:43:19PM +0900, Masanari Iida wrote: Correct spelling typo in debug message within drivers/usb. Signed-off-by: Masanari Iida standby2...@gmail.com This no longer applies, please redo it against the latest linux-next tree. When doing so, you can add my Acked-by: Felipe Balbi ba...@ti.com -- balbi signature.asc Description: Digital signature
Re: [PATCH] usb/gadget: let file storage gadget select libcomposite
Hi, On Wed, Oct 31, 2012 at 01:55:03PM +0100, Michal Nazarewicz wrote: On Wed, Oct 31 2012, Felipe Balbi ba...@ti.com wrote: nevertheless, I should be receiving a patch right about now removing that file. Alan, care to send a patch for that ? I have some other stuff to remove as well, so I can take care of it, so Alan can spend his time on something more important. that's up to you guys, I just need the patch removing what we committed to remove ;-) Either way is fine by me :-) cheers ps: thanks for volunteering :-) -- balbi signature.asc Description: Digital signature
Re: [PATCH 2/3] usb: gadget: fsl_qe_udc: do not use tasklet_disable before tasklet_kill
On Wed, Oct 31, 2012 at 04:06:00PM +0800, Xiaotian Feng wrote:
If tasklet_disable() is called before related tasklet handled,
tasklet_kill will never be finished. tasklet_kill is enough.
how did you test this ? Why changing FSL driver instead of switching
over to chipidea which is supposed to be shared by every licensee of the
chipidea core ?
Signed-off-by: Xiaotian Feng dannyf...@tencent.com
Cc: Li Yang le...@freescale.com
Cc: Felipe Balbi ba...@ti.com
Cc: Greg Kroah-Hartman gre...@linuxfoundation.org
Cc: linux-...@vger.kernel.org
Cc: linuxppc-...@lists.ozlabs.org
---
drivers/usb/gadget/fsl_qe_udc.c |4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/usb/gadget/fsl_qe_udc.c b/drivers/usb/gadget/fsl_qe_udc.c
index b09452d..4ad3b82 100644
--- a/drivers/usb/gadget/fsl_qe_udc.c
+++ b/drivers/usb/gadget/fsl_qe_udc.c
@@ -2661,7 +2661,7 @@ static int __devexit qe_udc_remove(struct
platform_device *ofdev)
usb_del_gadget_udc(udc-gadget);
udc-done = done;
- tasklet_disable(udc-rx_tasklet);
+ tasklet_kill(udc-rx_tasklet);
if (udc-nullmap) {
dma_unmap_single(udc-gadget.dev.parent,
@@ -2698,8 +2698,6 @@ static int __devexit qe_udc_remove(struct
platform_device *ofdev)
free_irq(udc-usb_irq, udc);
irq_dispose_mapping(udc-usb_irq);
- tasklet_kill(udc-rx_tasklet);
-
iounmap(udc-usb_regs);
device_unregister(udc-gadget.dev);
--
1.7.9.5
--
balbi
signature.asc
Description: Digital signature
Re: [PATCH V2 2/3] Remove VLAIS usage from gadget code
hi, On Tue, Oct 30, 2012 at 05:18:56PM -0400, Behan Webster wrote: The use of variable length arrays in structs (VLAIS) in the Linux Kernel code precludes the use of compilers which don't implement VLAIS (for instance the Clang compiler). This patch instead calculates offsets into the kmalloc-ed memory buffer using macros from valign.h. Signed-off-by: Behan Webster beh...@converseincode.com this won't apply after the current cleanups I applied to gadget code from Sebastian. If someone takes this patch, it will generate a series of annoying, hard-to-figure-out conflicts (at least judging by the looks of $SUBJECT). -- balbi signature.asc Description: Digital signature
Re: [PATCH] usb:musb: Dequeue urbs on device unplug
hi, On Mon, Oct 22, 2012 at 06:51:39AM +0200, Virupax SADASHIVPETIMATH wrote: -Original Message- From: Felipe Balbi [mailto:ba...@ti.com] Sent: Monday, October 15, 2012 5:49 PM To: Virupax SADASHIVPETIMATH Cc: ba...@ti.com; st...@rowland.harvard.edu; linux-...@vger.kernel.org; linux-kernel@vger.kernel.org; linus.wall...@linaro.org; Rajaram REGUPATHY; Praveena NADAHALLY Subject: Re: [PATCH] usb:musb: Dequeue urbs on device unplug Hi, On Wed, Oct 10, 2012 at 10:06:03AM +0530, Virupax Sadashivpetimath wrote: Flush queued urbs on receiving device disconnect interrupt. This is required for successful disconnect and successive enumeration of the device. In a failure case khubd hangs on usb-storage thread for completion. Seen in the below trace. [ 1355.764526] SysRq : Show Blocked State [ 1355.768341] taskPC stack pid father [ 1355.773620] khubd D c06a1fbc 0 503 2 0x [ 1355.780151] [c06a1fbc] (__schedule+0x3f0/0x8ec) from [c06a26a0] (schedule+0x58/0x70) [ 1355.788330] [c06a26a0] (schedule+0x58/0x70) from [c06a2af8] (schedule_timeout+0x1d8/0x31c) [ 1355.796997] [c06a2af8] (schedule_timeout+0x1d8/0x31c) from [c06a1994] (wait_for_common+0xd8/0x180) [ 1355.806396] [c06a1994] (wait_for_common+0xd8/0x180) from [c06a1b14] (wait_for_completion+0x20/0x24) [ 1355.815887] [c06a1b14] I would like to get some Tested-bys here. Can you please add Tested-by: shuai@stericsson.com as soon as you reply telling me how you tested. Which kind of testcases did you run ? thanks -- balbi signature.asc Description: Digital signature
Re: [PATCH] usb:musb: Dequeue urbs on device unplug
Hi,
On Wed, Oct 10, 2012 at 10:06:03AM +0530, Virupax Sadashivpetimath wrote:
Flush queued urbs on receiving device disconnect
interrupt. This is required for successful disconnect
and successive enumeration of the device.
In a failure case khubd hangs on usb-storage thread
for completion. Seen in the below trace.
[ 1355.764526] SysRq : Show Blocked State
[ 1355.768341] taskPC stack pid father
[ 1355.773620] khubd D c06a1fbc 0 503 2 0x
[ 1355.780151] [c06a1fbc] (__schedule+0x3f0/0x8ec) from [c06a26a0]
(schedule+0x58/0x70)
[ 1355.788330] [c06a26a0] (schedule+0x58/0x70) from [c06a2af8]
(schedule_timeout+0x1d8/0x31c)
[ 1355.796997] [c06a2af8] (schedule_timeout+0x1d8/0x31c) from [c06a1994]
(wait_for_common+0xd8/0x180)
[ 1355.806396] [c06a1994] (wait_for_common+0xd8/0x180) from [c06a1b14]
(wait_for_completion+0x20/0x24)
[ 1355.815887] [c06a1b14] (wait_for_completion+0x20/0x24) from [c00b9998]
(kthread_stop+0x68/0x17c)
[ 1355.825103] [c00b9998] (kthread_stop+0x68/0x17c) from [c039e0e0]
(release_everything+0x30/0x8c)
[ 1355.834228] [c039e0e0] (release_everything+0x30/0x8c) from [c039e168]
(usb_stor_disconnect+0x2c/0x30)
[ 1355.843902] [c039e168] (usb_stor_disconnect+0x2c/0x30) from [c038e3a8]
(usb_unbind_interface+0x60/0x1e0)
[ 1355.853820] [c038e3a8] (usb_unbind_interface+0x60/0x1e0) from
[c031dcc0] (__device_release_driver+0x80/0xd0)
[ 1355.864074] [c031dcc0] (__device_release_driver+0x80/0xd0) from
[c031ddfc] (device_release_driver+0x2c/0x38)
[ 1355.874359] [c031ddfc] (device_release_driver+0x2c/0x38) from
[c031d0d8] (bus_remove_device+0xbc/0x10c)
[ 1355.884155] [c031d0d8] (bus_remove_device+0xbc/0x10c) from [c031b63c]
(device_del+0x108/0x17c)
[ 1355.893188] [c031b63c] (device_del+0x108/0x17c) from [c038afe8]
(usb_disable_device+0xbc/0x200)
[ 1355.902313] [c038afe8] (usb_disable_device+0xbc/0x200) from [c0384c58]
(usb_disconnect+0xb8/0x194)
[ 1355.911682] [c0384c58] (usb_disconnect+0xb8/0x194) from [c0385e58]
(hub_thread+0x45c/0x14b0)
[ 1355.920562] [c0385e58] (hub_thread+0x45c/0x14b0) from [c00b97e0]
(kthread+0x98/0xa0)
[ 1355.928710] [c00b97e0] (kthread+0x98/0xa0) from [c0064834]
(kernel_thread_exit+0x0/0x8)
[ 1356.014373] usb-storage D c06a1fbc 0 2379 2 0x
[ 1356.020843] [c06a1fbc] (__schedule+0x3f0/0x8ec) from [c06a26a0]
(schedule+0x58/0x70)
[ 1356.029022] [c06a26a0] (schedule+0x58/0x70) from [c06a2af8]
(schedule_timeout+0x1d8/0x31c)
[ 1356.037719] [c06a2af8] (schedule_timeout+0x1d8/0x31c) from [c06a1994]
(wait_for_common+0xd8/0x180)
[ 1356.047088] [c06a1994] (wait_for_common+0xd8/0x180) from [c06a1b14]
(wait_for_completion+0x20/0x24)
[ 1356.056549] [c06a1b14] (wait_for_completion+0x20/0x24) from [c038b5dc]
(usb_sg_wait+0x108/0x194)
[ 1356.065795] [c038b5dc] (usb_sg_wait+0x108/0x194) from [c039d6dc]
(usb_stor_bulk_transfer_sglist+0x9c/0xf4)
[ 1356.075866] [c039d6dc] (usb_stor_bulk_transfer_sglist+0x9c/0xf4) from
[c039d76c] (usb_stor_bulk_srb+0x38/0x50)
[ 1356.086303] [c039d76c] (usb_stor_bulk_srb+0x38/0x50) from [c039d92c]
(usb_stor_Bulk_transport+0x114/0x2d0)
[ 1356.096374] [c039d92c] (usb_stor_Bulk_transport+0x114/0x2d0) from
[c039d190] (usb_stor_invoke_transport+0x34/0x3f4)
[ 1356.107238] [c039d190] (usb_stor_invoke_transport+0x34/0x3f4) from
[c039cc0c] (usb_stor_transparent_scsi_command+0x18/0x1c)
[ 1356.118804] [c039cc0c] (usb_stor_transparent_scsi_command+0x18/0x1c)
from [c039f144] (usb_stor_control_thread+0x190/0x28c)
[ 1356.130279] [c039f144] (usb_stor_control_thread+0x190/0x28c) from
[c00b97e0] (kthread+0x98/0xa0)
[ 1356.139465] [c00b97e0] (kthread+0x98/0xa0) from [c0064834]
(kernel_thread_exit+0x0/0x8)
Signed-off-by: Virupax Sadashivpetimath
virupax.sadashivpetim...@stericsson.com
Acked-by: Linus Walleij linus.wall...@linaro.org
---
drivers/usb/musb/musb_core.c | 37 +
drivers/usb/musb/musb_host.c |3 +++
2 files changed, 40 insertions(+), 0 deletions(-)
diff --git a/drivers/usb/musb/musb_core.c b/drivers/usb/musb/musb_core.c
index bb56a0e..fc6e990 100644
--- a/drivers/usb/musb/musb_core.c
+++ b/drivers/usb/musb/musb_core.c
@@ -414,6 +414,41 @@ static void musb_otg_timer_func(unsigned long data)
spin_unlock_irqrestore(musb-lock, flags);
}
+void musb_handle_disconnect(struct musb *musb)
missing static. Actually, why isn't this part of
musb_root_disconnect() ?? Any real reasoning behind it ?
Another point is that, if this function is really necessary, the name is
quite lame. It should be called musb_unlink_and_giveback_urbs() or
something similar.
+{
+ int epnum, i;
+ struct urb *urb;
+ struct musb_hw_ep *hw_ep;
+ struct musb_qh *qh;
+ struct usb_hcd *hcd = musb_to_hcd(musb);
+
+ for (epnum = 0; epnum musb-config-num_eps;
+ epnum++) {
+ hw_ep =
Re: [PATCH] usb: gadget: ncm: correct endianess conversion
On Sun, Oct 28, 2012 at 06:48:29PM +0100, Sebastian Andrzej Siewior wrote: On Sun, Oct 28, 2012 at 06:30:02PM +0100, Dmytro Milinevskyy wrote: I was trying to keep 2 tabs but checkpatch didn't accept long line that's why I killed extra tab. Then move them to the code section instead to initialize them in the declaration section. How does it work? Is the test on host side not strict enough? The host part(cdc_ncm) does not check this field. However I agree that at least on device side this should be corrected. Good. Felipe, do you want me to send another patch or resend previous one with this correction? If it hasn't been applied yet resend it. I think I hasn't applied this one yet, so resending is the way to go. -- balbi signature.asc Description: Digital signature
Re: [PATCH] usb: gadget: ncm: correct endianess conversion
Hi,
On Tue, Oct 30, 2012 at 10:44:27AM +0100, Dmytro Milinevskyy wrote:
Convert USB descriptor's fields to CPU byte order before using locally in USB
NCM gadget driver.
Tested on MIPS32 big-endian device.
Signed-off-by: Dmytro Milinevskyy milinevs...@gmail.com
---
drivers/usb/gadget/f_ncm.c | 12 ++--
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/usb/gadget/f_ncm.c b/drivers/usb/gadget/f_ncm.c
index b651b52..0b9e2ad 100644
--- a/drivers/usb/gadget/f_ncm.c
+++ b/drivers/usb/gadget/f_ncm.c
@@ -102,7 +102,7 @@ static inline unsigned ncm_bitrate(struct usb_gadget *g)
USB_CDC_NCM_NTB32_SUPPORTED)
static struct usb_cdc_ncm_ntb_parameters ntb_parameters = {
- .wLength = sizeof ntb_parameters,
+ .wLength = cpu_to_le16(sizeof(ntb_parameters)),
.bmNtbFormatsSupported = cpu_to_le16(FORMATS_SUPPORTED),
.dwNtbInMaxSize = cpu_to_le32(NTB_DEFAULT_IN_SIZE),
.wNdpInDivisor = cpu_to_le16(4),
@@ -869,15 +869,15 @@ static struct sk_buff *ncm_wrap_ntb(struct gether *port,
struct sk_buff *skb2;
int ncb_len = 0;
__le16 *tmp;
- int div = ntb_parameters.wNdpInDivisor;
- int rem = ntb_parameters.wNdpInPayloadRemainder;
- int pad;
- int ndp_align = ntb_parameters.wNdpInAlignment;
- int ndp_pad;
+ int div, rem, pad, ndp_align, ndp_pad;
I would not combine them in a single line. Keep them one per line.
cheers
--
balbi
signature.asc
Description: Digital signature
Re: [PATCH v3 1/3] drivers: bus: ocp2scp: add pdata support
On Sat, Oct 27, 2012 at 07:05:54PM +0530, Kishon Vijay Abraham I wrote:
ocp2scp was not having pdata support which makes *musb* fail for non-dt
boot in OMAP platform. The pdata will have information about the devices
that is connected to ocp2scp. ocp2scp driver will now make use of this
information to create the devices that is attached to ocp2scp.
Signed-off-by: Kishon Vijay Abraham I kis...@ti.com
just one small comment below. Other than that:
Acked-by: Felipe Balbi ba...@ti.com
---
drivers/bus/omap-ocp2scp.c | 67
++--
include/linux/platform_data/omap_ocp2scp.h | 31 +
2 files changed, 95 insertions(+), 3 deletions(-)
create mode 100644 include/linux/platform_data/omap_ocp2scp.h
diff --git a/drivers/bus/omap-ocp2scp.c b/drivers/bus/omap-ocp2scp.c
index ff63560..5db8297 100644
--- a/drivers/bus/omap-ocp2scp.c
+++ b/drivers/bus/omap-ocp2scp.c
@@ -22,6 +22,26 @@
#include linux/pm_runtime.h
#include linux/of.h
#include linux/of_platform.h
+#include linux/platform_data/omap_ocp2scp.h
+
+/**
+ * _count_resources - count for the number of resources
+ * @res: struct resource *
+ *
+ * Count and return the number of resources populated for the device that is
+ * connected to ocp2scp.
+ */
+static unsigned _count_resources(struct resource *res)
+{
+ int cnt = 0;
+
+ while (res-start != res-end) {
+ cnt++;
+ res++;
+ }
+
+ return cnt;
+}
static int ocp2scp_remove_devices(struct device *dev, void *c)
{
@@ -34,20 +54,61 @@ static int ocp2scp_remove_devices(struct device *dev,
void *c)
static int __devinit omap_ocp2scp_probe(struct platform_device *pdev)
{
- int ret;
- struct device_node *np = pdev-dev.of_node;
+ int ret;
+ unsigned res_cnt, i;
+ struct device_node *np = pdev-dev.of_node;
+ struct platform_device *pdev_child;
+ struct omap_ocp2scp_platform_data *pdata = pdev-dev.platform_data;
+ struct omap_ocp2scp_dev *dev;
if (np) {
ret = of_platform_populate(np, NULL, NULL, pdev-dev);
if (ret) {
- dev_err(pdev-dev, failed to add resources for
ocp2scp child\n);
+ dev_err(pdev-dev,
+ failed to add resources for ocp2scp child\n);
goto err0;
}
+ } else if (pdata) {
+ for (i = 0, dev = *pdata-devices; i pdata-dev_cnt; i++,
+ dev++) {
+ res_cnt = _count_resources(dev-res);
+
+ pdev_child = platform_device_alloc(dev-drv_name, -1);
please use PLATFORM_DEVID_AUTO instead.
--
balbi
signature.asc
Description: Digital signature
Re: [PATCH 2/3] usb: gadget: fsl_qe_udc: do not use tasklet_disable before tasklet_kill
Hi, On Wed, Oct 31, 2012 at 10:35:37PM +0800, Li Yang wrote: On Wed, Oct 31, 2012 at 9:26 PM, Felipe Balbi ba...@ti.com wrote: On Wed, Oct 31, 2012 at 04:06:00PM +0800, Xiaotian Feng wrote: If tasklet_disable() is called before related tasklet handled, tasklet_kill will never be finished. tasklet_kill is enough. how did you test this ? Why changing FSL driver instead of switching over to chipidea which is supposed to be shared by every licensee of the chipidea core ? The QE UDC is an private controller that is not compatible with the Chipidea core. thanks for the clarification, but you still haven't answered how you tested this ;-) -- balbi signature.asc Description: Digital signature
Re: [PATCH/Ver2] [trivial] usb: Fix typo in drivers/usb
Hi, On Thu, Nov 01, 2012 at 12:03:51AM +0900, Masanari Iida wrote: Correct spelling typo in debug messages within drivers/usb. Signed-off-by: Masanari Iida standby2...@gmail.com Acked-by: Felipe Balbi ba...@ti.com Greg, since now this only touches the stuff I handle, I can apply this myself. whatever works best for you, let me know. -- balbi signature.asc Description: Digital signature
Re: [PATCH] usb/gadget: let file storage gadget select libcomposite
Hi, On Wed, Oct 31, 2012 at 11:07:44AM -0400, Alan Stern wrote: On Wed, 31 Oct 2012, Michal Nazarewicz wrote: On Wed, Oct 31 2012, Felipe Balbi ba...@ti.com wrote: nevertheless, I should be receiving a patch right about now removing that file. Alan, care to send a patch for that ? I have some other stuff to remove as well, so I can take care of it, so Alan can spend his time on something more important. I'm happy to have Michal remove g_file_storage. That way, he gets to deal with any complaints. :-) heh, good point :-) -- balbi signature.asc Description: Digital signature
Re: [PATCH v2 1/4] i2c: introduce i2c-cbus driver
On Wed, Oct 31, 2012 at 08:03:43PM +0200, Aaro Koskinen wrote:
Add i2c driver to enable access to devices behind CBUS on Nokia Internet
Tablets.
The patch also adds CBUS I2C configuration for N8x0 which is one of the
users of this driver.
Cc: linux-...@vger.kernel.org
Acked-by: Felipe Balbi ba...@ti.com
Acked-by: Tony Lindgren t...@atomide.com
Signed-off-by: Aaro Koskinen aaro.koski...@iki.fi
---
arch/arm/mach-omap2/board-n8x0.c | 42 ++
drivers/i2c/busses/Kconfig | 10 ++
drivers/i2c/busses/Makefile |1 +
drivers/i2c/busses/i2c-cbus.c| 300
++
include/linux/i2c-cbus.h | 27
5 files changed, 380 insertions(+), 0 deletions(-)
create mode 100644 drivers/i2c/busses/i2c-cbus.c
create mode 100644 include/linux/i2c-cbus.h
diff --git a/arch/arm/mach-omap2/board-n8x0.c
b/arch/arm/mach-omap2/board-n8x0.c
index d95f727..7ea0348 100644
--- a/arch/arm/mach-omap2/board-n8x0.c
+++ b/arch/arm/mach-omap2/board-n8x0.c
@@ -16,8 +16,10 @@
#include linux/gpio.h
#include linux/init.h
#include linux/io.h
+#include linux/irq.h
#include linux/stddef.h
#include linux/i2c.h
+#include linux/i2c-cbus.h
#include linux/spi/spi.h
#include linux/usb/musb.h
#include linux/platform_data/spi-omap2-mcspi.h
@@ -39,6 +41,45 @@
#define TUSB6010_GPIO_ENABLE 0
#define TUSB6010_DMACHAN 0x3f
+#if defined(CONFIG_I2C_CBUS) || defined(CONFIG_I2C_CBUS_MODULE)
+static struct i2c_cbus_platform_data n8x0_cbus_data = {
+ .clk_gpio = 66,
+ .dat_gpio = 65,
+ .sel_gpio = 64,
+};
+
+static struct platform_device n8x0_cbus_device = {
+ .name = i2c-cbus,
+ .id = 3,
+ .dev= {
+ .platform_data = n8x0_cbus_data,
+ },
+};
+
+static struct i2c_board_info n8x0_i2c_board_info_3[] __initdata = {
+ {
+ I2C_BOARD_INFO(retu-mfd, 0x01),
+ },
+};
+
+static void __init n8x0_cbus_init(void)
+{
+ const int retu_irq_gpio = 108;
+
+ if (gpio_request_one(retu_irq_gpio, GPIOF_IN, Retu IRQ))
+ return;
+ irq_set_irq_type(gpio_to_irq(retu_irq_gpio), IRQ_TYPE_EDGE_RISING);
+ n8x0_i2c_board_info_3[0].irq = gpio_to_irq(retu_irq_gpio);
+ i2c_register_board_info(3, n8x0_i2c_board_info_3,
+ ARRAY_SIZE(n8x0_i2c_board_info_3));
+ platform_device_register(n8x0_cbus_device);
+}
+#else /* CONFIG_I2C_CBUS */
+static void __init n8x0_cbus_init(void)
+{
+}
+#endif /* CONFIG_I2C_CBUS */
+
#if defined(CONFIG_USB_MUSB_TUSB6010) ||
defined(CONFIG_USB_MUSB_TUSB6010_MODULE)
/*
* Enable or disable power to TUSB6010. When enabling, turn on 3.3 V and
@@ -677,6 +718,7 @@ static void __init n8x0_init_machine(void)
gpmc_onenand_init(board_onenand_data);
n8x0_mmc_init();
n8x0_usb_init();
+ n8x0_cbus_init();
}
MACHINE_START(NOKIA_N800, Nokia N800)
diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
index 65dd599..d01c8ef 100644
--- a/drivers/i2c/busses/Kconfig
+++ b/drivers/i2c/busses/Kconfig
@@ -338,6 +338,16 @@ config I2C_BLACKFIN_TWI_CLK_KHZ
help
The unit of the TWI clock is kHz.
+config I2C_CBUS
+ tristate CBUS I2C driver
+ depends on GENERIC_GPIO
+ help
+ Support for CBUS access using I2C API. Mostly relevant for Nokia
+ Internet Tablets (770, N800 and N810).
+
+ This driver can also be built as a module. If so, the module
+ will be called i2c-cbus.
+
config I2C_CPM
tristate Freescale CPM1 or CPM2 (MPC8xx/826x)
depends on (CPM1 || CPM2) OF_I2C
diff --git a/drivers/i2c/busses/Makefile b/drivers/i2c/busses/Makefile
index 2d33d62..3c548b1 100644
--- a/drivers/i2c/busses/Makefile
+++ b/drivers/i2c/busses/Makefile
@@ -31,6 +31,7 @@ obj-$(CONFIG_I2C_POWERMAC) += i2c-powermac.o
obj-$(CONFIG_I2C_AT91) += i2c-at91.o
obj-$(CONFIG_I2C_AU1550) += i2c-au1550.o
obj-$(CONFIG_I2C_BLACKFIN_TWI) += i2c-bfin-twi.o
+obj-$(CONFIG_I2C_CBUS) += i2c-cbus.o
obj-$(CONFIG_I2C_CPM)+= i2c-cpm.o
obj-$(CONFIG_I2C_DAVINCI)+= i2c-davinci.o
obj-$(CONFIG_I2C_DESIGNWARE_CORE)+= i2c-designware-core.o
diff --git a/drivers/i2c/busses/i2c-cbus.c b/drivers/i2c/busses/i2c-cbus.c
new file mode 100644
index 000..1ea7667
--- /dev/null
+++ b/drivers/i2c/busses/i2c-cbus.c
@@ -0,0 +1,300 @@
+/*
+ * CBUS I2C driver for Nokia Internet Tablets.
+ *
+ * Copyright (C) 2004-2010 Nokia Corporation
+ *
+ * Based on code written by Juha Yrjölä, David Weinehall, Mikko Ylinen and
+ * Felipe Balbi. Converted to I2C driver by Aaro Koskinen.
+ *
+ * This file is subject to the terms and conditions of the GNU General
+ * Public License. See the file COPYING in the main directory of this
+ * archive for more details.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY
Re: [PATCH v2 2/4] mfd: introduce retu-mfd driver
On Wed, Oct 31, 2012 at 08:03:44PM +0200, Aaro Koskinen wrote:
Retu is a multi-function device found on Nokia Internet Tablets
implementing at least watchdog, RTC, headset detection and power button
functionality.
This patch implements minimum functionality providing register access,
IRQ handling and power off functions.
Cc: sa...@linux.intel.com
Acked-by: Felipe Balbi ba...@ti.com
Acked-by: Tony Lindgren t...@atomide.com
Signed-off-by: Aaro Koskinen aaro.koski...@iki.fi
---
drivers/mfd/Kconfig |9 ++
drivers/mfd/Makefile |1 +
drivers/mfd/retu-mfd.c | 264
++
include/linux/mfd/retu.h | 22
4 files changed, 296 insertions(+), 0 deletions(-)
create mode 100644 drivers/mfd/retu-mfd.c
create mode 100644 include/linux/mfd/retu.h
diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index acab3ef..7528c5e 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -1044,6 +1044,15 @@ config MFD_PALMAS
If you say yes here you get support for the Palmas
series of PMIC chips from Texas Instruments.
+config MFD_RETU
+ tristate Support for Retu multi-function device
+ select MFD_CORE
+ depends on I2C
+ select REGMAP_IRQ
+ help
+ Retu is a multi-function device found on Nokia Internet Tablets
+ (770, N800 and N810).
+
endmenu
endif
diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
index d8ccb63..ad7879f 100644
--- a/drivers/mfd/Makefile
+++ b/drivers/mfd/Makefile
@@ -138,3 +138,4 @@ obj-$(CONFIG_MFD_RC5T583) += rc5t583.o rc5t583-irq.o
obj-$(CONFIG_MFD_SEC_CORE) += sec-core.o sec-irq.o
obj-$(CONFIG_MFD_SYSCON) += syscon.o
obj-$(CONFIG_MFD_LM3533) += lm3533-core.o lm3533-ctrlbank.o
+obj-$(CONFIG_MFD_RETU) += retu-mfd.o
diff --git a/drivers/mfd/retu-mfd.c b/drivers/mfd/retu-mfd.c
new file mode 100644
index 000..8f81566
--- /dev/null
+++ b/drivers/mfd/retu-mfd.c
@@ -0,0 +1,264 @@
+/*
+ * Retu MFD driver
+ *
+ * Copyright (C) 2004, 2005 Nokia Corporation
+ *
+ * Based on code written by Juha Yrjölä, David Weinehall and Mikko Ylinen.
+ * Rewritten by Aaro Koskinen.
+ *
+ * This file is subject to the terms and conditions of the GNU General
+ * Public License. See the file COPYING in the main directory of this
+ * archive for more details.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ */
+
+#include linux/err.h
+#include linux/i2c.h
+#include linux/irq.h
+#include linux/init.h
+#include linux/slab.h
+#include linux/mutex.h
+#include linux/module.h
+#include linux/regmap.h
+#include linux/mfd/core.h
+#include linux/mfd/retu.h
+#include linux/interrupt.h
+#include linux/moduleparam.h
+
+/* Registers */
+#define RETU_REG_ASICR 0x00/* ASIC ID and revision
*/
+#define RETU_REG_ASICR_VILMA (1 7)/* Bit indicating Vilma */
+#define RETU_REG_IDR 0x01/* Interrupt ID */
+#define RETU_REG_IMR 0x02/* Interrupt mask */
+
+/* Interrupt sources */
+#define RETU_INT_PWR 0 /* Power button */
+
+struct retu_dev {
+ struct regmap *regmap;
+ struct device *dev;
+ struct mutexmutex;
+ struct regmap_irq_chip_data *irq_data;
+};
+
+static struct resource retu_pwrbutton_res[] = {
+ {
+ .name = retu-pwrbutton,
+ .start = RETU_INT_PWR,
+ .end= RETU_INT_PWR,
+ .flags = IORESOURCE_IRQ,
+ },
+};
+
+static struct mfd_cell retu_devs[] = {
+ {
+ .name = retu-wdt
+ },
+ {
+ .name = retu-pwrbutton,
+ .resources = retu_pwrbutton_res[0],
.resources = retu_powerbutton_res, ??
--
balbi
signature.asc
Description: Digital signature
Re: [PATCH 0/3] capebus moving omap_devices to mach-omap2
Hi, On Wed, Oct 31, 2012 at 11:36:25PM +0200, Pantelis Antoniou wrote: * Pantelis Antoniou pa...@antoniou-consulting.com [121031 13:14]: On Oct 31, 2012, at 9:55 PM, Benoit Cousson wrote: Yeah, I do agree. I'm confused as well. Only OMAP IPs under PRCM control could have an hwmod and thus must be handled by an omap_device. Any devices that is created later cannot be omap_device. The DT core will create regular platform_device for them. Since cape is an external board, it should have nothing to do with omap_device. Looking at your patch: da8xx-dt: Create da8xx DT adapter device I understand why you do that, but in fact that patch does not make sense to me :-( Why do you have to create an omap_device from the driver probe? The problem is that capes are not external boards in the normal sense as a PCI board is. In the PCI case the hardware that implements the desired functionality is on the PCI board, while in the cape case the hardware module is in the SoC. The cape most of the times is quite simple and contains passive components, leds or some kind of I2C/SPI devices. Ah now I see, you're talking about the beaglebone extension boards :) The way to deal with those properly is to just edit the board .dts entry to include omap-beaglebone-cape-xyz.dtsi whatever. That is what is being done... This is the fallout. You can't instantiate the omap_device early in the boot process like it was done up to now in the board file. You can only do that later in the boot process (for built-in cape drivers), or even after user-space has booted and the matching cape driver module has been loaded. Yes you can, just edit the .dts file for the extension board you have connected. This stuff should be DT only for sure, let's not even start adding platform_data entries for that. The omap_device entries for the omap internal devices will be created automatically during startup based on the .dts. Note that you can set status = disabled for the omap internal devices in the .dts file, the devices are there in the hardware. If you are sure the extension boards are safely hot pluggable, then all you need is some interface to enable and disable devices after booting. But that should be done in Linux generic way. So we should not export any omap_hwmod or omap_device things, and want to keep it __init only, and local to arch/arm/mach-omap2. I disagree. This is not something that is handled by just editing the dts. The way it is handled, is in the Linux generic way, we have a proper bus, and the drivers as compiled as standalone file. when you have an actual bus, that'd be correct. We're hitting a use case that wasn't there in omap until now. There a a whole bunch of conflicting capes. There's no way to instantiate them together. They must be instantiated only after their EEPROMs are read and they are matched to their corresponding cape drivers. why this requirement of instantiating them only after reading EEPROMs ? It's unnecessary to add that requirement, just do what Tony said (include my-awesome-cape.dtsi to bone.dts) and all i2c/spi devices should be created automatically. The thing is that there is no such thing as a cape-device. The cape is just a collection of I2C, 1-wire and SPI devices anyway. What we should instantiate is bmp085, tls2550, sht21, instead of instantiating beagle-bone-weather-cape. What's the problem in just instantiating all of those from bone.dts ? Expose the EEPROMs to userland so whatever SW you guys have running on the bone can figure out what features to expose to the SDK which the user sees, but the kernel doesn't need to know that there is a weather-cape attached to the bone. -- balbi signature.asc Description: Digital signature
Re: [PATCH V2 2/3] Remove VLAIS usage from gadget code
Hi, On Wed, Oct 31, 2012 at 11:33:20AM -0400, Behan Webster wrote: On 12-10-31 09:28 AM, Felipe Balbi wrote: hi, On Tue, Oct 30, 2012 at 05:18:56PM -0400, Behan Webster wrote: The use of variable length arrays in structs (VLAIS) in the Linux Kernel code precludes the use of compilers which don't implement VLAIS (for instance the Clang compiler). This patch instead calculates offsets into the kmalloc-ed memory buffer using macros from valign.h. Signed-off-by: Behan Webster beh...@converseincode.com this won't apply after the current cleanups I applied to gadget code from Sebastian. Makes sense. I'll try it with your repo, and regenerate. If someone takes this patch, it will generate a series of annoying, hard-to-figure-out conflicts (at least judging by the looks of $SUBJECT). I just tried the patch on your git.kernel.org repo and thankfully there is only one hunk which is rejected, and fortunately the reason is trivial (descriptors - fs_descriptors). Was: -func-function.descriptors = data-fs_descs; +func-function.descriptors = fs_descs; Now is: -func-function.fs_descriptors = data-fs_descs; +func-function.fs_descriptors = fs_descs; I will regenerate the patch set, but obviously the new gadget patch in the V3 patchset will only apply to the USB repo, and not to the netfilter repo. then we can merge to net tree and handle the conflicts when merging to Linus, that'd be fine by me as long as people know how to solve the conflict properly ;-) -- balbi signature.asc Description: Digital signature
Re: [PATCH 0/3] capebus moving omap_devices to mach-omap2
Hi,
On Thu, Nov 01, 2012 at 12:39:30PM +0200, Pantelis Antoniou wrote:
lcd@0 {
compatible = adafruit,tft-lcd-1.8-red,
sitronix,st7735;
spi-max-frequency = 800;
reg = 0;
spi-cpol;
spi-cpha;
pinctrl-names = default;
pinctrl-0 = lcd_pins;
st7735-rst = gpio4 19 0;
st7735-dc = gpio4 21 0;
};
};
};
I guess there is no easy solution for that, but it looks to me that
what you have to do is to make the DT creation dynamic in your case.
Assuming you do not want to do that in the bootloader, you must do
that pretty early during the boot process to end up with a full
description of your DT tree before creating the devices.
Do it pretty early in the boot processes ended up with the am335xevm board
file in the PSP tree.
The whole set of possible cape designs cannot be controlled, nor do we want
to.
We want to empower users to come up with their own designs without having to
do any kernel/boot loader
hacking.
that's impossible since you will have to provide the capebus driver
anyway.
Each cape will have their own DTS and based on some board id you
will fix the DT dynamically.
My point is that the issue you are facing is a real limitation of
DT, so you should fix the DT core and not workaround it by creating
artificial bindings / drivers.
You still haven't described any mechanism to deal with all the use
cases I described.
DT can't and will not deal with the complexity that we're facing right
now.
and DT-itself shouldn't. I agree with Benoit that this should be built
at bootloader level, perhaps. Whatever you're doing in capebus, you
could do at kernel space, build your DT bindings in runtime, and pass
that DT blob to kernel.
One question though, what do you mean by some capes are full blown
devices with their own drivers ? Do you mean you have capes running
some other (RT)OS and communicating with linux somehow ? How does it
communicate to the bone ?
cheers
--
balbi
signature.asc
Description: Digital signature
Re: [PATCH 0/3] capebus moving omap_devices to mach-omap2
Hi,
On Thu, Nov 01, 2012 at 01:26:10PM +0200, Pantelis Antoniou wrote:
Hi
On Nov 1, 2012, at 1:04 PM, Felipe Balbi wrote:
Hi,
On Thu, Nov 01, 2012 at 12:39:30PM +0200, Pantelis Antoniou wrote:
lcd@0 {
compatible = adafruit,tft-lcd-1.8-red,
sitronix,st7735;
spi-max-frequency = 800;
reg = 0;
spi-cpol;
spi-cpha;
pinctrl-names = default;
pinctrl-0 = lcd_pins;
st7735-rst = gpio4 19 0;
st7735-dc = gpio4 21 0;
};
};
};
I guess there is no easy solution for that, but it looks to me that
what you have to do is to make the DT creation dynamic in your case.
Assuming you do not want to do that in the bootloader, you must do
that pretty early during the boot process to end up with a full
description of your DT tree before creating the devices.
Do it pretty early in the boot processes ended up with the am335xevm board
file in the PSP tree.
The whole set of possible cape designs cannot be controlled, nor do we
want to.
We want to empower users to come up with their own designs without having
to do any kernel/boot loader
hacking.
that's impossible since you will have to provide the capebus driver
anyway.
The generic cape bus driver can handle the easy stuff. More complex
stuff can be supported with per-cape drivers.
likewise:
the generic stuff can be handled by FDT easily, the more complex stuff
can be supported by dynamically changing FDT blob, no ?
Each cape will have their own DTS and based on some board id you
will fix the DT dynamically.
My point is that the issue you are facing is a real limitation of
DT, so you should fix the DT core and not workaround it by creating
artificial bindings / drivers.
You still haven't described any mechanism to deal with all the use
cases I described.
DT can't and will not deal with the complexity that we're facing right
now.
and DT-itself shouldn't. I agree with Benoit that this should be built
at bootloader level, perhaps. Whatever you're doing in capebus, you
could do at kernel space, build your DT bindings in runtime, and pass
that DT blob to kernel.
We're just passing the buck someplace else. We're not fixing the problem.
What makes you think that dealing with this in the bootloader is going
to be simpler?
I never said it was supposed to be simpler, it just doesn't sound like
something the kernel should care about. Kernel cares about the
One question though, what do you mean by some capes are full blown
devices with their own drivers ? Do you mean you have capes running
some other (RT)OS and communicating with linux somehow ? How does it
communicate to the bone ?
Some have FPGAs.
https://specialcomp.com/beaglebone/BeagleBone_FPGA.html
how does linux communicate with those ? What they are matters very
little ;-) All we need is an interface to load binaries to the FPGA.
Some capes have their own MCUs.
A recent one has an 6502 communicating with uio_pruss.
https://github.com/ohporter/b6502
that uses remoteproc, so I assume it's using OMAP's mailbox ?
Again I say that this is not a 'capebus', it's just another device
which we use another interface to talk to.
i2c devices will be children of the omap i2c controller, spi devices
will be children of the omap mcspi, GPMC devices will need the GPMC
controller and so on, but none of this looks like argument to introduce
a fake bus into the kernel.
--
balbi
signature.asc
Description: Digital signature
Re: [PATCH 0/3] capebus moving omap_devices to mach-omap2
Hi, On Thu, Nov 01, 2012 at 01:00:21PM +0100, Koen Kooi wrote: tl;dr: please suggest an actual solution that allows plugplay when plugging in multiple capes and applying power after that. Preferably one that doesn't pass the buck to u-boot. I can think of a few ways: 1) ship your distribution with all necessary drivers and let udev handle driver probing. clearly this will require constant updates for the distribution as new capes are developed. But IIUC, that's the same with Arduino where new libraries are added to the Arduino OS. 2) ship with drivers for EEPROMs only and setup a repository of drivers this would require every driver to be packaged separately, then you create a setup where bone's userland will use EEPROMs data to figure out which drivers it needs, pass that information to SDK via USB, then SDK downloads all necessary/missing packages, sends them to bone via USB and all packages are installed on the bone. Note that since packages would be 'installed', this would be a one-time-only thing. 3) realize that if your user can build an FPGA cape, s/he can most likely write code and adding/recompiling kernel shouldn't be the biggest of his/her worries (no comments to this one, I understand it's not feasible) in any case, capebus sounds like something which is hugely unnecessary. ps: at least for the I2C subsystem, i2c-core can detect for you if your driver provides -detect() method. Op 1 nov. 2012, om 12:26 heeft Cousson, Benoit b-cous...@ti.com het volgende geschreven: FWIW, we do have a similar, but simpler, problem with the beagle / beagle-xm and panda / panda-es. But for the moment we are just using a different DTS for each board. A different DTS for each board combination... There alot more capes for the bone that they are for the beagle the panda. And more are going to come, but not necessarily from people that have any connection to TI or CCO. Sure, but my point is that your solution will not solve our problem :-) This is a generic problem that you address with a very custom / specific approach. You still haven't described how I could solve the issue of conflicting capes using a single DTS. Well I don't have the solution otherwise I will have done it already :-) My point is that the solution has to be in the DT core if not in the bootloader. snarky commentSo when we at beagleboard.org handled the beagleboard and beagleboard xM expansionboards in the bootloader (detection, muxing, etc) we were told it was wrong and we should handle it in the kernel and if we waited a bit, DT would solve everything. And now that we actually handle it in the kernel you are saying that it is wrong and we should handle it in the bootloader and that DT won't solve everything. I guess someone will now tell us that uEFI will fix everything./snarky comment Apart from that, you and others still fail to tell us how you want to handle a user (or customer) that buys a beaglebone, an LCD cape, a weatherstation cape and a geiger counter cape and plugs those together and powers them on. With the evil TI vendor tree and extra patches the boardfile reads the eeproms and tries its best to instantiate all the platform data. One of the capes won't have working LEDs since appending to the leds-gpio struct is pretty much impossible in this couldn't you just instantiate multiple leds-gpio device ? situation. But it gets a picture on the screen, blinks LEDs and does largely what the user expects. With capebus we get: 1) da8xx-fb which lacks DT bindingsp[1] receives plaftorm data to match the LCD this is something which could be fixed at the driver level, right ? :-) 2) the i2c sensors on the weathercape are registered that will work with or without capebus. 3) the one-wire bus on the weathercape gets registered also should work with or without capebus. 4) the LEDs on the lcd cape get registered5 also works with or without capebus. 5) the LED on the geigercape gets registered and adds a custom trigger also works with or without capebus. 6) the ADC, which again, lacks DT bindings[2], receives plaftorm data and a custom IIO binding driver problem. 7) pinctrl sets the pinmuxes for the above also works with or without capebus. In other words: plug play, even for devices with drivers that are still lacking DT support. I _do_ agree with you that we could have a grace period where DT and non-DT would boot together, but apparently that's not something which isn't trivial to do. I guess Benoit might also be concerned that if we add such an infrastructure than conversion to DT will never finish heh. Now please explain to me why you think it's such an awesome idea that the users will need to find the right dtsi files (multiple revisions of the lcd cape exist), include them in the dts, run dtc, add a
Re: [PATCH 0/3] capebus moving omap_devices to mach-omap2
Hi, On Thu, Nov 01, 2012 at 02:57:26PM +0200, Pantelis Antoniou wrote: Each cape will have their own DTS and based on some board id you will fix the DT dynamically. My point is that the issue you are facing is a real limitation of DT, so you should fix the DT core and not workaround it by creating artificial bindings / drivers. You still haven't described any mechanism to deal with all the use cases I described. DT can't and will not deal with the complexity that we're facing right now. and DT-itself shouldn't. I agree with Benoit that this should be built at bootloader level, perhaps. Whatever you're doing in capebus, you could do at kernel space, build your DT bindings in runtime, and pass that DT blob to kernel. We're just passing the buck someplace else. We're not fixing the problem. What makes you think that dealing with this in the bootloader is going to be simpler? I never said it was supposed to be simpler, it just doesn't sound like something the kernel should care about. Kernel cares about the I just disagree here. The kernel should provide services that make the life of users easier, not the lives of the kernel developers easier. and it's already doing that isn't it ? we have i2c framework for i2c clients. From a userland point of view, you have input layer, iio, hwmon, chardev and a whole bunch of other interfaces which standardize device accesses. Look at your sentence again: kernel should make life of users easier, not that of kernel developers; IMHO capebus is just making it easier for the kernel developers who have to maintain a bunch of drivers for different devices on different capes ;-) At the end of the day, capebus will just be creating devices which will be handled by the same I2C framework, iio, input, hwmon, and so on. So what was the great benefit from capebus other than decreasing the amount of changes to .dts files ? One question though, what do you mean by some capes are full blown devices with their own drivers ? Do you mean you have capes running some other (RT)OS and communicating with linux somehow ? How does it communicate to the bone ? Some have FPGAs. https://specialcomp.com/beaglebone/BeagleBone_FPGA.html how does linux communicate with those ? What they are matters very little ;-) All we need is an interface to load binaries to the FPGA. We can not know what people will come up with. It might have a few GPIOs to load the bitfile to the FPGA, but after that, no-one can tell. I might not interface to Linux at all; it might interface via I2C, or RS232. which means that whoever writes RTL for the FPGA needs to do so with bone's I/O choices in mind. Let's assume the use UART to send bitfile to FPGA and bitfile is a model of an I2C Sensor, they'll have to use /dev/ttyOn to pass bitfile to FPGA and later an i2c-client driver (possibly using iio, since it's a sensor) will be loaded. Right ? Chances are, it won't fit in any kind of known drivers of linux. Some guy is using an FPGA for SDR, another is making a radar cape. awesome. That means we need to take care of those :-) Even with capebus, they will still have to write those drivers won't they ? So instead of writing some capebus driver, why can't the guy write a driver for his radar instead ? That way, if he ever turns that into an ASIC and decides to sell it as a chip, he doesn't have to write another driver just to strip the capebus away. These guys don't give a damn frankly about Linux. What they do care about is having a cheap/easy to develop platform for their own little widget. If you are going to ask from the to hunt down their own dts and assemble from various dtsi's they'll just move to something else. I never asked that :-) What I'm claiming is that capebus doesn't sound like the best solution for the problem exposed. Which is a shame, cause we do have a nice platform here. I agree with you, the bone is quite awesome ;-) Some capes have their own MCUs. A recent one has an 6502 communicating with uio_pruss. https://github.com/ohporter/b6502 that uses remoteproc, so I assume it's using OMAP's mailbox ? Again I say that this is not a 'capebus', it's just another device which we use another interface to talk to. i2c devices will be children of the omap i2c controller, spi devices will be children of the omap mcspi, GPMC devices will need the GPMC controller and so on, but none of this looks like argument to introduce a fake bus into the kernel. I'll let the users of said bus to do the talking. You're just flat out wrong IMO. fair enough, I feel the same way about your capebus ;-) -- balbi signature.asc Description: Digital signature
Re: [PATCH 0/3] capebus moving omap_devices to mach-omap2
HI, On Thu, Nov 01, 2012 at 03:59:50PM +0200, Pantelis Antoniou wrote: Hi Alan, On Nov 1, 2012, at 3:51 PM, Alan Cox wrote: What they want, and what every user wants, is I plug this board in, and the driver make sure everything is loaded and ready. No, the end users don't want to see any of the implementation details of how the bitfile is transported; the driver can handle it. That doesn't necessarily make it a bus merely some kind of hotplug enumeration of devices. That should all work properly both for devices and busses with spi and i²c as the final bits needed for it got fixed some time ago. In an ideal world you don't want to be writing custom drivers for stuff. If your cape routes an i²c serial device to the existing system i²c busses then you want to just create an instance of any existing driver on the existing i²c bus not create a whole new layer of goop. It does need to do the plumbing and resource management for the plumbing but thats not the same as being a bus. Alan Fair enough. But there's no such thing a 'hotplug enumeration construct' in Linux yet, and a bus is the closest thing to it. It does take advantage of the nice way device code matches drivers and devices though. I'm afraid that having the I2C/SPI drivers doing the detection won't work. The capes can have arbitrary I2C/SPI devices (and even more weird components). There is no way to assure for example that the I2C device responding to address 'foo' in cape A is the same I2C device responding to the same address in cape B. your -detect() method should take care of that. -- balbi signature.asc Description: Digital signature
Re: [PATCH 0/3] capebus moving omap_devices to mach-omap2
Hi, On Thu, Nov 01, 2012 at 04:49:23PM -0700, Russ Dill wrote: On Thu, Nov 1, 2012 at 3:05 PM, Felipe Balbi ba...@ti.com wrote: HI, On Thu, Nov 01, 2012 at 03:59:50PM +0200, Pantelis Antoniou wrote: Hi Alan, On Nov 1, 2012, at 3:51 PM, Alan Cox wrote: What they want, and what every user wants, is I plug this board in, and the driver make sure everything is loaded and ready. No, the end users don't want to see any of the implementation details of how the bitfile is transported; the driver can handle it. That doesn't necessarily make it a bus merely some kind of hotplug enumeration of devices. That should all work properly both for devices and busses with spi and i²c as the final bits needed for it got fixed some time ago. In an ideal world you don't want to be writing custom drivers for stuff. If your cape routes an i²c serial device to the existing system i²c busses then you want to just create an instance of any existing driver on the existing i²c bus not create a whole new layer of goop. It does need to do the plumbing and resource management for the plumbing but thats not the same as being a bus. Alan Fair enough. But there's no such thing a 'hotplug enumeration construct' in Linux yet, and a bus is the closest thing to it. It does take advantage of the nice way device code matches drivers and devices though. I'm afraid that having the I2C/SPI drivers doing the detection won't work. The capes can have arbitrary I2C/SPI devices (and even more weird components). There is no way to assure for example that the I2C device responding to address 'foo' in cape A is the same I2C device responding to the same address in cape B. your -detect() method should take care of that. There isn't some magical serial number in I²C devices that a -detect() method can read and the implementation of I²C is somewhat flexible. One devices read may be another devices write. A detect look at what other drivers do. You can read a revision register, you can write a command and see if the device responds as expected, it doesn't matter. method that only performs reads could easily toggle a gpio that resets the board, rewrite and eeprom, or set the printer on fire. If you how ? It's just a read. browse through various detect functions, yes, some of them key off an ID, but a lot of them just check various registers to see if certain bits are zero, or certain bits are one. A lot of I²C devices I've dealt with have no good way of probing them, especially GPIO chips (you'll notice none of the I²C GPIO expanders have detect functions) it doesn't mean it can't be done. On top of all this the detect routine does not tell you if the alert pin is connected to some IRQ, or in the case of a GPIO expander, what those GPIOs are connected to, etc, etc. so what ? All you want to do with detect is figure out if the far end is who you think it is, not what it's doing. -- balbi signature.asc Description: Digital signature
Re: linux-next: build warning after merge of the usb tree
Hi, On Fri, Nov 02, 2012 at 03:08:28PM +1100, Stephen Rothwell wrote: Hi Greg, After merging the usb tree, today's linux-next build (x86_64 allmodconfig) produced this warning: WARNING: drivers/usb/host/ehci-hcd: 'ehci_init_driver' exported twice. Previous export was in drivers/usb/chipidea/ci_hdrc.ko WARNING: drivers/usb/host/ehci-hcd: 'ehci_resume' exported twice. Previous export was in drivers/usb/chipidea/ci_hdrc.ko WARNING: drivers/usb/host/ehci-hcd: 'ehci_suspend' exported twice. Previous export was in drivers/usb/chipidea/ci_hdrc.ko WARNING: drivers/usb/host/ehci-hcd: 'ehci_setup' exported twice. Previous export was in drivers/usb/chipidea/ci_hdrc.ko Introduced by commit 3e0232039967 (USB: EHCI: prepare to make ehci-hcd a library module). Alex, why on earth is chipidea exporting symbols it doesn't own ? -- balbi signature.asc Description: Digital signature
Re: [PATCH 0/3] capebus moving omap_devices to mach-omap2
Hi, On Fri, Nov 02, 2012 at 02:42:51AM -0700, Russ Dill wrote: browse through various detect functions, yes, some of them key off an ID, but a lot of them just check various registers to see if certain bits are zero, or certain bits are one. A lot of I²C devices I've dealt with have no good way of probing them, especially GPIO chips (you'll notice none of the I²C GPIO expanders have detect functions) it doesn't mean it can't be done. Really? Please, do tell how you would write a detect function for a PCA9534. It has 4 registers, an input port registers, an output port register, a polarity inversion register, and a configuration register. read them and match to their reset values, perhaps ? And don't forget, since we are probing, every detect routine for every I²C driver will have to run with every I²C address on every bus, possibly with both address formats. not *every* I2C address. What you say is wrong, a -detect() method will only run for those addresses which the device can actually assume. On top of all this the detect routine does not tell you if the alert pin is connected to some IRQ, or in the case of a GPIO expander, what those GPIOs are connected to, etc, etc. so what ? All you want to do with detect is figure out if the far end is who you think it is, not what it's doing. If we already knew who was there, we wouldn't need a detect routine. of course not :-) But the whole discussion has been about not knowing which capes (and thus which devices) are attached to the bone. -- balbi signature.asc Description: Digital signature
Re: [RFT/PATCH] serial: omap: prevent resume if device is not suspended.
On Tue, Sep 25, 2012 at 09:30:29AM +0100, Russell King - ARM Linux wrote: On Tue, Sep 25, 2012 at 01:52:03PM +0530, Poddar, Sourav wrote: Hi Greg, Ping on this? On Tue, Sep 18, 2012 at 6:10 PM, Sourav Poddar sourav.pod...@ti.com wrote: Greg's tty-next is not booting on 2420 based N800. The failure is observed at serial init itself. The reason might be that n800 tries to resume even though it is not suspended before. How is this happening? I think that needs proper investigation - or if it's had more investigation, then the results needs to be included in the commit description so that everyone can understand the issue here. We should not be resuming a device which hasn't been suspended. Maybe the runtime PM enable sequence is wrong, and that's what should be fixed instead? This sequence in the probe() function: pm_runtime_irq_safe(pdev-dev); pm_runtime_enable(pdev-dev); pm_runtime_get_sync(pdev-dev); would enable runtime PM while the s/w state indicates that it's disabled, and then that pm_runtime_get_sync() will want to resume the device. See the section 5. Runtime PM Initialization, Device Probing and Removal in Documentation/power/runtime_pm.txt, specifically the second paragraph of that section. that was tested. It worked in pandaboard but didn't work on beagleboard XM. Sourav tried to start a discussion about that, but it simply died... In any case, pm_runtime_get_sync() in probe will always call runtime_resume callback, right ? -- balbi signature.asc Description: Digital signature
Re: [RFT/PATCH] serial: omap: prevent resume if device is not suspended.
On Tue, Sep 25, 2012 at 10:12:28AM +0100, Russell King - ARM Linux wrote: On Tue, Sep 25, 2012 at 11:31:20AM +0300, Felipe Balbi wrote: On Tue, Sep 25, 2012 at 09:30:29AM +0100, Russell King - ARM Linux wrote: How is this happening? I think that needs proper investigation - or if it's had more investigation, then the results needs to be included in the commit description so that everyone can understand the issue here. We should not be resuming a device which hasn't been suspended. Maybe the runtime PM enable sequence is wrong, and that's what should be fixed instead? This sequence in the probe() function: pm_runtime_irq_safe(pdev-dev); pm_runtime_enable(pdev-dev); pm_runtime_get_sync(pdev-dev); would enable runtime PM while the s/w state indicates that it's disabled, and then that pm_runtime_get_sync() will want to resume the device. See the section 5. Runtime PM Initialization, Device Probing and Removal in Documentation/power/runtime_pm.txt, specifically the second paragraph of that section. that was tested. It worked in pandaboard but didn't work on beagleboard XM. Sourav tried to start a discussion about that, but it simply died... In any case, pm_runtime_get_sync() in probe will always call runtime_resume callback, right ? Well, if the runtime PM state says it's suspended, and then you enable runtime PM, the first call to pm_runtime_get_sync() will trigger a resume attempt. The patch description is complaining about resume events without there being a preceding suspend event. This could well be why. that's most likely, of course. But should we cause a regression to beagleboard XM because of that ? Also, if you look into chapter 9 of the runtime_pm documentation, starting on line 822 you'll see documentation suggests the use of mystruct-is_suspended flag. -- balbi signature.asc Description: Digital signature
Re: [RFT/PATCH] serial: omap: prevent resume if device is not suspended.
Hi, On Tue, Sep 25, 2012 at 10:21:18AM +0100, Russell King - ARM Linux wrote: On Tue, Sep 25, 2012 at 12:11:14PM +0300, Felipe Balbi wrote: On Tue, Sep 25, 2012 at 10:12:28AM +0100, Russell King - ARM Linux wrote: On Tue, Sep 25, 2012 at 11:31:20AM +0300, Felipe Balbi wrote: On Tue, Sep 25, 2012 at 09:30:29AM +0100, Russell King - ARM Linux wrote: How is this happening? I think that needs proper investigation - or if it's had more investigation, then the results needs to be included in the commit description so that everyone can understand the issue here. We should not be resuming a device which hasn't been suspended. Maybe the runtime PM enable sequence is wrong, and that's what should be fixed instead? This sequence in the probe() function: pm_runtime_irq_safe(pdev-dev); pm_runtime_enable(pdev-dev); pm_runtime_get_sync(pdev-dev); would enable runtime PM while the s/w state indicates that it's disabled, and then that pm_runtime_get_sync() will want to resume the device. See the section 5. Runtime PM Initialization, Device Probing and Removal in Documentation/power/runtime_pm.txt, specifically the second paragraph of that section. that was tested. It worked in pandaboard but didn't work on beagleboard XM. Sourav tried to start a discussion about that, but it simply died... In any case, pm_runtime_get_sync() in probe will always call runtime_resume callback, right ? Well, if the runtime PM state says it's suspended, and then you enable runtime PM, the first call to pm_runtime_get_sync() will trigger a resume attempt. The patch description is complaining about resume events without there being a preceding suspend event. This could well be why. that's most likely, of course. But should we cause a regression to beagleboard XM because of that ? What would cause a regression on beagleboard XM? I have not suggested any change other than more investigation of the issue and a fuller patch description - yet you're screaming (idiotically IMHO) that mere investigation would break beagleboard. Well, if it's _that_ fragile, that mere investigation of this issue by someone elsewhere on the planet would break your beagleboard, maybe it deserves to be broken! why are you always so over the top like that ? This is just counter-productive to say the least. What I'm trying to say here, is that there might be a bug either on pm core or on OMAP3's implementation and I'd like to get input from Tony, Santosh, Paul, etc before going forward. Maybe it's something they've already been through, or maybe it rings a bell and points to somewhere we should look for. anyway, instead of feeding your ego, we can stop this discussion and let Sourav see what he can come up with. -- balbi signature.asc Description: Digital signature
Re: [RFT/PATCH] serial: omap: prevent resume if device is not suspended.
On Tue, Sep 25, 2012 at 11:29:58AM +0100, Russell King - ARM Linux wrote: On Tue, Sep 25, 2012 at 12:48:16PM +0300, Felipe Balbi wrote: Hi, On Tue, Sep 25, 2012 at 10:21:18AM +0100, Russell King - ARM Linux wrote: On Tue, Sep 25, 2012 at 12:11:14PM +0300, Felipe Balbi wrote: On Tue, Sep 25, 2012 at 10:12:28AM +0100, Russell King - ARM Linux wrote: On Tue, Sep 25, 2012 at 11:31:20AM +0300, Felipe Balbi wrote: On Tue, Sep 25, 2012 at 09:30:29AM +0100, Russell King - ARM Linux wrote: How is this happening? I think that needs proper investigation - or if it's had more investigation, then the results needs to be included in the commit description so that everyone can understand the issue here. We should not be resuming a device which hasn't been suspended. Maybe the runtime PM enable sequence is wrong, and that's what should be fixed instead? This sequence in the probe() function: pm_runtime_irq_safe(pdev-dev); pm_runtime_enable(pdev-dev); pm_runtime_get_sync(pdev-dev); would enable runtime PM while the s/w state indicates that it's disabled, and then that pm_runtime_get_sync() will want to resume the device. See the section 5. Runtime PM Initialization, Device Probing and Removal in Documentation/power/runtime_pm.txt, specifically the second paragraph of that section. that was tested. It worked in pandaboard but didn't work on beagleboard XM. Sourav tried to start a discussion about that, but it simply died... In any case, pm_runtime_get_sync() in probe will always call runtime_resume callback, right ? Well, if the runtime PM state says it's suspended, and then you enable runtime PM, the first call to pm_runtime_get_sync() will trigger a resume attempt. The patch description is complaining about resume events without there being a preceding suspend event. This could well be why. that's most likely, of course. But should we cause a regression to beagleboard XM because of that ? What would cause a regression on beagleboard XM? I have not suggested any change other than more investigation of the issue and a fuller patch description - yet you're screaming (idiotically IMHO) that mere investigation would break beagleboard. Well, if it's _that_ fragile, that mere investigation of this issue by someone elsewhere on the planet would break your beagleboard, maybe it deserves to be broken! why are you always so over the top like that ? This is just counter-productive to say the least. Because you are accusing me of potentially breaking your beagleboard for merely suggesting further investigation and a better commit message. Where did I accuse you of anyting ? I just mentioned we experienced a regression with beagleboard XM when using pm_runtime_set_active(). here's my quote: that was tested. It worked in pandaboard but didn't work on beagleboard XM. Sourav tried to start a discussion about that, but it simply died... To add extra info, here you go: We pinged Paul and asked if he had seen that before, he had no pointers... Because Documentation/power/runtime_pm.txt was using a mystruct-is_suspended flag, we just decided to follow the same design since no-one was able to suggest why pm_runtime_set_active() was breaking beagleXM nor how it was supposed to actually work. Reading the code: pm_runtime_set_active() would tell pm_runtime core the device is actually active by setting runtime_status to RPM_ACTIVE, thus the following pm_runtime_get_sync() wouldn't actually call runtime_resume() callback, but it would increment usage_counter. I can't see why this would fail on beagleXM, but it does and we'd like to hear in which situations this could fail... -- balbi signature.asc Description: Digital signature
Re: [PATCHv2 1/7] rtc: Convert struct i2c_msg initialization to C99 format
On Tue, Sep 25, 2012 at 04:03:38PM +0530, Shubhrajyoti D wrote:
Convert the struct i2c_msg initialization to C99 format. This makes
maintaining and editing the code simpler. Also helps once other fields
like transferred are added in future.
Signed-off-by: Shubhrajyoti D shubhrajy...@ti.com
---
drivers/rtc/rtc-ds1672.c | 26 ++
1 files changed, 22 insertions(+), 4 deletions(-)
diff --git a/drivers/rtc/rtc-ds1672.c b/drivers/rtc/rtc-ds1672.c
index 7fa67d0..577dbae 100644
--- a/drivers/rtc/rtc-ds1672.c
+++ b/drivers/rtc/rtc-ds1672.c
@@ -37,8 +37,17 @@ static int ds1672_get_datetime(struct i2c_client *client,
struct rtc_time *tm)
unsigned char buf[4];
struct i2c_msg msgs[] = {
- {client-addr, 0, 1, addr},/* setup read ptr */
- {client-addr, I2C_M_RD, 4, buf}, /* read date */
+ {/* setup read ptr */
+ .addr = client-addr,
+ .len = 1,
+ .buf = addr
+ },
+ {/* setup read ptr */
should this comment be /* read date */ ??
--
balbi
signature.asc
Description: Digital signature
Re: [RFT/PATCH] serial: omap: prevent resume if device is not suspended.
Hi, On Tue, Sep 25, 2012 at 12:07:03PM +0100, Russell King - ARM Linux wrote: On Tue, Sep 25, 2012 at 01:37:03PM +0300, Felipe Balbi wrote: On Tue, Sep 25, 2012 at 11:29:58AM +0100, Russell King - ARM Linux wrote: Because you are accusing me of potentially breaking your beagleboard for merely suggesting further investigation and a better commit message. Where did I accuse you of anyting ? I just mentioned we experienced a regression with beagleboard XM when using pm_runtime_set_active(). I quote: : But should we cause a regression to beagleboard XM because of that ? maybe that wasn't the best way to put it, but I was trying to point out that either there was a bug on pm core or omap_device/hwmod, not that we shouldn't investigate. I say again: I was _only_ suggesting further investigation, yet you were mouthing off about causing a regression to beagleboard because of that, effectively saying that no, we should not do any further investigation and this is the only fix. not what I meant, but fair enough... The because of that was supposed to mean because of pm_runtime_set_active(). I find the documentation for that particular call to be rather poor and a bit confusing, specially when further down, the very same document uses a is_suspended flag which, in fact, shouldn't be needed when we have pm_runtime_is_suspended() and the like. We pinged Paul and asked if he had seen that before, he had no pointers... Because Documentation/power/runtime_pm.txt was using a mystruct-is_suspended flag, we just decided to follow the same design since no-one was able to suggest why pm_runtime_set_active() was breaking beagleXM nor how it was supposed to actually work. Reading the code: pm_runtime_set_active() would tell pm_runtime core the device is actually active by setting runtime_status to RPM_ACTIVE, thus the following pm_runtime_get_sync() wouldn't actually call runtime_resume() callback, but it would increment usage_counter. I can't see why this would fail on beagleXM, but it does and we'd like to hear in which situations this could fail... Well, I've just spent five minutes analysing the code paths - which I hadn't looked at before - and I've pointed out what's probably causing the problem for Beagle. I think you owe me an appology over your aggressive attitude towards my suggestions that there needed to be some further investigation. I don't see any aggressive attitude towards what you suggested, actually. Mailing list archives are available to check, but the one cursing around was always yourself and THAT deserves an apology. If you still think I've been at all aggressive, then sure, I apologize, it wasn't what I meant though. -- balbi signature.asc Description: Digital signature
Re: [PATCHv3 0/7] rtc: convert to c99 format
Hi, On Tue, Sep 25, 2012 at 05:01:59PM +0530, Shubhrajyoti D wrote: The series tries to convert the i2c_msg to c99 struct. This may avoid issues like below if someone tries to add a structure. http://www.mail-archive.com/linux-i2c@vger.kernel.org/msg08972.html Special thanks to Julia Lawall for helping it automate. By the below script. http://www.mail-archive.com/cocci@diku.dk/msg02753.html Resending with Andrew Morton in cc for review as it was suggested to me. Fix a typo in the comments that crept in. Shubhrajyoti D (7): rtc: Convert struct i2c_msg initialization to C99 format rtc: Convert struct i2c_msg initialization to C99 format rtc: Convert struct i2c_msg initialization to C99 format rtc: Convert struct i2c_msg initialization to C99 format rtc: Convert struct i2c_msg initialization to C99 format rtc: Convert struct i2c_msg initialization to C99 format rtc: Convert struct i2c_msg initialization to C99 format FWIW: Reviewed-by: Felipe Balbi ba...@ti.com complete series look fine. -- balbi signature.asc Description: Digital signature
Re: [PATCH 4/4] usb: phy: omap-usb2: enable 960Mhz clock for omap5
Hi,
On Wed, Sep 26, 2012 at 11:10:48AM +0530, ABRAHAM, KISHON VIJAY wrote:
Hi,
On Wed, Sep 19, 2012 at 5:26 PM, Felipe Balbi ba...@ti.com wrote:
On Wed, Sep 19, 2012 at 05:00:29PM +0530, Kishon Vijay Abraham I wrote:
usb_otg_ss_refclk960m is needed by usb2 phy present in omap5. For
omap4, the clk_get of this clock will fail since it does not have this
clock.
Signed-off-by: Kishon Vijay Abraham I kis...@ti.com
---
Documentation/devicetree/bindings/usb/usb-phy.txt |3 +++
drivers/usb/phy/omap-usb2.c | 28
-
2 files changed, 30 insertions(+), 1 deletion(-)
diff --git a/Documentation/devicetree/bindings/usb/usb-phy.txt
b/Documentation/devicetree/bindings/usb/usb-phy.txt
index 7c5fd89..d5626de 100644
--- a/Documentation/devicetree/bindings/usb/usb-phy.txt
+++ b/Documentation/devicetree/bindings/usb/usb-phy.txt
@@ -24,6 +24,9 @@ Required properties:
add the address of control module phy power register until a driver for
control module is added
+Optional properties:
+ - has960mhzclk: should be added if the phy needs 960mhz clock
+
This is usually a subnode of ocp2scp to which it is connected.
usb3phy@4a084400 {
diff --git a/drivers/usb/phy/omap-usb2.c b/drivers/usb/phy/omap-usb2.c
index d36c282..d6612ba 100644
--- a/drivers/usb/phy/omap-usb2.c
+++ b/drivers/usb/phy/omap-usb2.c
@@ -146,6 +146,7 @@ static int __devinit omap_usb2_probe(struct
platform_device *pdev)
struct omap_usb *phy;
struct usb_otg *otg;
struct resource *res;
+ struct device_node *np = pdev-dev.of_node;
phy = devm_kzalloc(pdev-dev, sizeof(*phy), GFP_KERNEL);
if (!phy) {
@@ -190,6 +191,15 @@ static int __devinit omap_usb2_probe(struct
platform_device *pdev)
}
clk_prepare(phy-wkupclk);
+ if (of_property_read_bool(np, has960mhzclk)) {
+ phy-optclk = devm_clk_get(phy-dev,
usb_otg_ss_refclk960m);
+ if (IS_ERR(phy-optclk)) {
+ dev_err(pdev-dev, unable to get refclk960m\n);
+ return PTR_ERR(phy-optclk);
+ }
+ clk_prepare(phy-optclk);
+ }
instead, can't you just always try to get the clock but ignore the error
if it fails ?
This clock is needed for usb2 to work in dwc3 (omap5). So we have to
report the error in case we dont get the clock no?
sure, but you don't need to bail out. Print a warning message such as:
dev_dbg(pdev-dev, couldn't get refclk960m, trying without\n);
or something similar. Then you don't need to add this has960mhzclk flag
to dts files.
--
balbi
signature.asc
Description: Digital signature
Re: [PATCH 4/4] usb: phy: omap-usb2: enable 960Mhz clock for omap5
On Thu, Sep 27, 2012 at 10:43:06AM +0530, ABRAHAM, KISHON VIJAY wrote:
Hi,
On Wed, Sep 26, 2012 at 11:57 PM, Felipe Balbi ba...@ti.com wrote:
Hi,
On Wed, Sep 26, 2012 at 11:10:48AM +0530, ABRAHAM, KISHON VIJAY wrote:
Hi,
On Wed, Sep 19, 2012 at 5:26 PM, Felipe Balbi ba...@ti.com wrote:
On Wed, Sep 19, 2012 at 05:00:29PM +0530, Kishon Vijay Abraham I wrote:
usb_otg_ss_refclk960m is needed by usb2 phy present in omap5. For
omap4, the clk_get of this clock will fail since it does not have this
clock.
Signed-off-by: Kishon Vijay Abraham I kis...@ti.com
---
Documentation/devicetree/bindings/usb/usb-phy.txt |3 +++
drivers/usb/phy/omap-usb2.c | 28
-
2 files changed, 30 insertions(+), 1 deletion(-)
diff --git a/Documentation/devicetree/bindings/usb/usb-phy.txt
b/Documentation/devicetree/bindings/usb/usb-phy.txt
index 7c5fd89..d5626de 100644
--- a/Documentation/devicetree/bindings/usb/usb-phy.txt
+++ b/Documentation/devicetree/bindings/usb/usb-phy.txt
@@ -24,6 +24,9 @@ Required properties:
add the address of control module phy power register until a driver for
control module is added
+Optional properties:
+ - has960mhzclk: should be added if the phy needs 960mhz clock
+
This is usually a subnode of ocp2scp to which it is connected.
usb3phy@4a084400 {
diff --git a/drivers/usb/phy/omap-usb2.c b/drivers/usb/phy/omap-usb2.c
index d36c282..d6612ba 100644
--- a/drivers/usb/phy/omap-usb2.c
+++ b/drivers/usb/phy/omap-usb2.c
@@ -146,6 +146,7 @@ static int __devinit omap_usb2_probe(struct
platform_device *pdev)
struct omap_usb *phy;
struct usb_otg *otg;
struct resource *res;
+ struct device_node *np = pdev-dev.of_node;
phy = devm_kzalloc(pdev-dev, sizeof(*phy), GFP_KERNEL);
if (!phy) {
@@ -190,6 +191,15 @@ static int __devinit omap_usb2_probe(struct
platform_device *pdev)
}
clk_prepare(phy-wkupclk);
+ if (of_property_read_bool(np, has960mhzclk)) {
+ phy-optclk = devm_clk_get(phy-dev,
usb_otg_ss_refclk960m);
+ if (IS_ERR(phy-optclk)) {
+ dev_err(pdev-dev, unable to get refclk960m\n);
+ return PTR_ERR(phy-optclk);
+ }
+ clk_prepare(phy-optclk);
+ }
instead, can't you just always try to get the clock but ignore the error
if it fails ?
This clock is needed for usb2 to work in dwc3 (omap5). So we have to
report the error in case we dont get the clock no?
sure, but you don't need to bail out. Print a warning message such as:
dev_dbg(pdev-dev, couldn't get refclk960m, trying without\n);
but then we'll get this debug message for omap4 which actually doesn't
need 960m clk.
then make it dev_vdbg(). It's just a debugging message, it's not saying
it will fail, it's just stating that clock isn't present and we're
trying without because it's known that some versions don't need that
clock.
Another way to do it, would be to request or not the extra clock, based
on the Revision Register (if this IP has one).
--
balbi
signature.asc
Description: Digital signature
Re: [PATCHv3 1/7] dmaengine: dw_dmac: use helper macro module_platform_driver()
On Thu, Sep 27, 2012 at 10:31:55AM +0300, Andy Shevchenko wrote:
From: Heikki Krogerus heikki.kroge...@linux.intel.com
commit log would be great.
Signed-off-by: Heikki Krogerus heikki.kroge...@linux.intel.com
Signed-off-by: Andy Shevchenko andriy.shevche...@linux.intel.com
---
drivers/dma/dw_dmac.c | 13 ++---
1 file changed, 2 insertions(+), 11 deletions(-)
diff --git a/drivers/dma/dw_dmac.c b/drivers/dma/dw_dmac.c
index c4b0eb3..0b88ced 100644
--- a/drivers/dma/dw_dmac.c
+++ b/drivers/dma/dw_dmac.c
@@ -1700,6 +1700,7 @@ MODULE_DEVICE_TABLE(of, dw_dma_id_table);
#endif
static struct platform_driver dw_driver = {
+ .probe = dw_probe,
probe's in __init section. This is wrong. You need to change probe
__devinit.
.remove = __devexit_p(dw_remove),
.shutdown = dw_shutdown,
.driver = {
@@ -1709,17 +1710,7 @@ static struct platform_driver dw_driver = {
},
};
-static int __init dw_init(void)
-{
- return platform_driver_probe(dw_driver, dw_probe);
-}
-subsys_initcall(dw_init);
-
-static void __exit dw_exit(void)
-{
- platform_driver_unregister(dw_driver);
-}
-module_exit(dw_exit);
+module_platform_driver(dw_driver);
MODULE_LICENSE(GPL v2);
MODULE_DESCRIPTION(Synopsys DesignWare DMA Controller driver);
--
1.7.10.4
--
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/
--
balbi
signature.asc
Description: Digital signature
Re: [PATCHv3 3/7] dmaengine: dw_dmac: remove CLK dependency
Hi, On Thu, Sep 27, 2012 at 10:31:57AM +0300, Andy Shevchenko wrote: From: Heikki Krogerus heikki.kroge...@linux.intel.com This driver could be used on different platforms. Thus, the HAVE_CLK dependency is dropped away. Signed-off-by: Heikki Krogerus heikki.kroge...@linux.intel.com Signed-off-by: Andy Shevchenko andriy.shevche...@linux.intel.com --- drivers/dma/Kconfig |1 - 1 file changed, 1 deletion(-) diff --git a/drivers/dma/Kconfig b/drivers/dma/Kconfig index 677cd6e..df32537 100644 --- a/drivers/dma/Kconfig +++ b/drivers/dma/Kconfig @@ -83,7 +83,6 @@ config INTEL_IOP_ADMA config DW_DMAC tristate Synopsys DesignWare AHB DMA support - depends on HAVE_CLK as is, this will break compilation of any arch which doesn't set HAVE_CLK. -- balbi signature.asc Description: Digital signature
Re: [PATCHv3 5/7] dmaengine: dw_dmac: add PCI part of the driver
On Thu, Sep 27, 2012 at 10:31:59AM +0300, Andy Shevchenko wrote:
From: Heikki Krogerus heikki.kroge...@linux.intel.com
This is the PCI part of the DesignWare DMAC driver. The controller is usually
used in the Intel hardware such as Medfield.
Signed-off-by: Heikki Krogerus heikki.kroge...@linux.intel.com
Signed-off-by: Andy Shevchenko andriy.shevche...@linux.intel.com
---
drivers/dma/Kconfig |9
drivers/dma/Makefile |1 +
drivers/dma/dw_dmac_pci.c | 126
+
3 files changed, 136 insertions(+)
create mode 100644 drivers/dma/dw_dmac_pci.c
diff --git a/drivers/dma/Kconfig b/drivers/dma/Kconfig
index df32537..a5c7f64 100644
--- a/drivers/dma/Kconfig
+++ b/drivers/dma/Kconfig
@@ -89,6 +89,15 @@ config DW_DMAC
Support the Synopsys DesignWare AHB DMA controller. This
can be integrated in chips such as the Atmel AT32ap7000.
+config DW_DMAC_PCI
+ tristate Synopsys DesignWare AHB DMA support (PCI bus)
+ depends on PCI
+ select DW_DMAC
+ help
+ Support the Synopsys DesignWare AHB DMA controller on the platfroms
+ that provide it as a PCI device. For example, Intel Medfield has
+ integrated this GPDMA controller.
+
config AT_HDMAC
tristate Atmel AHB DMA support
depends on ARCH_AT91
diff --git a/drivers/dma/Makefile b/drivers/dma/Makefile
index 7428fea..15eef5f 100644
--- a/drivers/dma/Makefile
+++ b/drivers/dma/Makefile
@@ -12,6 +12,7 @@ obj-$(CONFIG_FSL_DMA) += fsldma.o
obj-$(CONFIG_MPC512X_DMA) += mpc512x_dma.o
obj-$(CONFIG_MV_XOR) += mv_xor.o
obj-$(CONFIG_DW_DMAC) += dw_dmac.o
+obj-$(CONFIG_DW_DMAC_PCI) += dw_dmac_pci.o
obj-$(CONFIG_AT_HDMAC) += at_hdmac.o
obj-$(CONFIG_MX3_IPU) += ipu/
obj-$(CONFIG_TXX9_DMAC) += txx9dmac.o
diff --git a/drivers/dma/dw_dmac_pci.c b/drivers/dma/dw_dmac_pci.c
new file mode 100644
index 000..b7ad37f
--- /dev/null
+++ b/drivers/dma/dw_dmac_pci.c
@@ -0,0 +1,126 @@
+/*
+ * PCI driver for the Synopsys DesignWare DMA Controller
+ *
+ * Copyright (C) 2012 Intel Corporation
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include linux/module.h
+#include linux/pci.h
+#include linux/platform_device.h
+#include linux/dw_dmac.h
+
+static struct dw_dma_platform_data pdata = {
pdata looks like it wants a prefix.
+ .is_private = 1,
+ .chan_allocation_order = CHAN_ALLOCATION_ASCENDING,
+ .chan_priority = CHAN_PRIORITY_ASCENDING,
+};
This is the same for all of the PCI IDs listed below, looks like it's
best to just add it as platform_data of the dwc_dmac device directly,
rather than passing a pointer to this via driver-data.
+
+static int __devinit dw_pci_probe(struct pci_dev *pdev,
+ const struct pci_device_id *id)
+{
+ struct platform_device *pd;
+ struct resource r[2];
+ struct dw_dma_platform_data *driver = (void *)id-driver_data;
missing space after the cast... but it looks unnecessary
+ static int instance;
this could be a IDA instead.
--
balbi
signature.asc
Description: Digital signature
Re: [PATCHv3 6/7] dma: move dw_dmac driver to an own directory
On Thu, Sep 27, 2012 at 10:32:00AM +0300, Andy Shevchenko wrote:
The dw_dmac driver contains multiple files. To make a managment of them more
typo: 'management'
convenient move it to an own directory.
Signed-off-by: Andy Shevchenko andriy.shevche...@linux.intel.com
---
drivers/dma/Makefile|3 +--
drivers/dma/dw/Makefile |2 ++
drivers/dma/{ = dw}/dw_dmac.c |2 +-
drivers/dma/{ = dw}/dw_dmac_pci.c |0
drivers/dma/{ = dw}/dw_dmac_regs.h |0
5 files changed, 4 insertions(+), 3 deletions(-)
create mode 100644 drivers/dma/dw/Makefile
rename drivers/dma/{ = dw}/dw_dmac.c (99%)
rename drivers/dma/{ = dw}/dw_dmac_pci.c (100%)
rename drivers/dma/{ = dw}/dw_dmac_regs.h (100%)
diff --git a/drivers/dma/Makefile b/drivers/dma/Makefile
index 15eef5f..122a48a 100644
--- a/drivers/dma/Makefile
+++ b/drivers/dma/Makefile
@@ -11,8 +11,7 @@ obj-$(CONFIG_INTEL_IOP_ADMA) += iop-adma.o
obj-$(CONFIG_FSL_DMA) += fsldma.o
obj-$(CONFIG_MPC512X_DMA) += mpc512x_dma.o
obj-$(CONFIG_MV_XOR) += mv_xor.o
-obj-$(CONFIG_DW_DMAC) += dw_dmac.o
-obj-$(CONFIG_DW_DMAC_PCI) += dw_dmac_pci.o
+obj-$(CONFIG_DW_DMAC) += dw/
obj-$(CONFIG_AT_HDMAC) += at_hdmac.o
obj-$(CONFIG_MX3_IPU) += ipu/
obj-$(CONFIG_TXX9_DMAC) += txx9dmac.o
diff --git a/drivers/dma/dw/Makefile b/drivers/dma/dw/Makefile
new file mode 100644
index 000..2edfb24
--- /dev/null
+++ b/drivers/dma/dw/Makefile
@@ -0,0 +1,2 @@
+obj-$(CONFIG_DW_DMAC) += dw_dmac.o
+obj-$(CONFIG_DW_DMAC_PCI) += dw_dmac_pci.o
diff --git a/drivers/dma/dw_dmac.c b/drivers/dma/dw/dw_dmac.c
similarity index 99%
rename from drivers/dma/dw_dmac.c
rename to drivers/dma/dw/dw_dmac.c
index 9f0129d..fa0471a 100644
--- a/drivers/dma/dw_dmac.c
+++ b/drivers/dma/dw/dw_dmac.c
@@ -24,8 +24,8 @@
#include linux/platform_device.h
#include linux/slab.h
+#include ../dmaengine.h
#include dw_dmac_regs.h
-#include dmaengine.h
/*
* This supports the Synopsys DesignWare AHB Central DMA Controller,
diff --git a/drivers/dma/dw_dmac_pci.c b/drivers/dma/dw/dw_dmac_pci.c
similarity index 100%
rename from drivers/dma/dw_dmac_pci.c
rename to drivers/dma/dw/dw_dmac_pci.c
diff --git a/drivers/dma/dw_dmac_regs.h b/drivers/dma/dw/dw_dmac_regs.h
similarity index 100%
rename from drivers/dma/dw_dmac_regs.h
rename to drivers/dma/dw/dw_dmac_regs.h
--
1.7.10.4
--
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/
--
balbi
signature.asc
Description: Digital signature
Re: [PATCHv3 7/7] MAINTAINERS: add recently created files to dw_dmac section
On Thu, Sep 27, 2012 at 10:32:01AM +0300, Andy Shevchenko wrote: Append myself to the mail entry of the section as well. Signed-off-by: Andy Shevchenko andriy.shevche...@linux.intel.com --- MAINTAINERS |4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/MAINTAINERS b/MAINTAINERS index 7dfd0eb..b87cbb1d 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -5999,10 +5999,10 @@ F:drivers/tty/serial SYNOPSYS DESIGNWARE DMAC DRIVER M: Viresh Kumar viresh.li...@gmail.com +M: Andy Shevchenko andriy.shevche...@linux.intel.com this needs to be agreed with Viresh first, I guess. Not sure if it was done or not. If it was, sorry for the noise ;-) -- balbi signature.asc Description: Digital signature
Re: [PATCHv3 4/7] dmaengine: dw_dmac: amend description and indentation
On Thu, Sep 27, 2012 at 10:31:58AM +0300, Andy Shevchenko wrote: From: Heikki Krogerus heikki.kroge...@linux.intel.com The driver will be used as a core part for various implementations of the DesignWare DMA device. The patch adjusts description on the top and corrects paragraph indentation in few places across the code. Signed-off-by: Heikki Krogerus heikki.kroge...@linux.intel.com Signed-off-by: Andy Shevchenko andriy.shevche...@linux.intel.com --- drivers/dma/dw_dmac.c |7 --- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/drivers/dma/dw_dmac.c b/drivers/dma/dw_dmac.c index bbb2a82..9f0129d 100644 --- a/drivers/dma/dw_dmac.c +++ b/drivers/dma/dw_dmac.c @@ -1,14 +1,15 @@ /* - * Driver for the Synopsys DesignWare DMA Controller (aka DMACA on - * AVR32 systems.) + * Core driver for the Synopsys DesignWare DMA Controller * * Copyright (C) 2007-2008 Atmel Corporation * Copyright (C) 2010-2011 ST Microelectronics + * Copyright (C) 2012 Intel Corporation I'm not a lawyer, but I'm not sure the few changes done to this driver is enough for Intel to hold a copyright here... dunno, though. -- balbi signature.asc Description: Digital signature
Re: [PATCHv3 3/7] dmaengine: dw_dmac: remove CLK dependency
Hi, On Thu, Sep 27, 2012 at 11:04:54AM +0300, Andy Shevchenko wrote: On Thu, Sep 27, 2012 at 10:42 AM, Felipe Balbi ba...@ti.com wrote: - depends on HAVE_CLK as is, this will break compilation of any arch which doesn't set HAVE_CLK. As Viresh suggested and I checked it's not. He wrote nice patch that adds stubs for such case. Great, that finally reached mainline. I was waiting for that patch for a long time, actually :-) thanks for the note -- balbi signature.asc Description: Digital signature
Re: [PATCHv3 5/7] dmaengine: dw_dmac: add PCI part of the driver
Hi, On Thu, Sep 27, 2012 at 11:08:12AM +0300, Andy Shevchenko wrote: + .is_private = 1, + .chan_allocation_order = CHAN_ALLOCATION_ASCENDING, + .chan_priority = CHAN_PRIORITY_ASCENDING, +}; This is the same for all of the PCI IDs listed below, looks like it's best to just add it as platform_data of the dwc_dmac device directly, rather than passing a pointer to this via driver-data. It potentially could be altered. I prefer to leave it as structure in the pci driver. fair enough ;-) -- balbi signature.asc Description: Digital signature
Re: [PATCHv3 1/7] dmaengine: dw_dmac: use helper macro module_platform_driver()
On Thu, Sep 27, 2012 at 01:46:17PM +0530, viresh kumar wrote:
On Thu, Sep 27, 2012 at 1:10 PM, Felipe Balbi ba...@ti.com wrote:
diff --git a/drivers/dma/dw_dmac.c b/drivers/dma/dw_dmac.c
@@ -1700,6 +1700,7 @@ MODULE_DEVICE_TABLE(of, dw_dma_id_table);
#endif
static struct platform_driver dw_driver = {
+ .probe = dw_probe,
probe's in __init section. This is wrong. You need to change probe
__devinit.
Good one. How can i miss it. :(
@Andy: Please add my Acked-by: after this change.
after this change you can also add:
Reviewed-by: Felipe Balbi ba...@ti.com
--
balbi
signature.asc
Description: Digital signature
Re: [PATCHv3 2/7] dmaengine: dw_dmac: add module alias
On Thu, Sep 27, 2012 at 01:46:57PM +0530, viresh kumar wrote:
On Thu, Sep 27, 2012 at 1:01 PM, Andy Shevchenko
andriy.shevche...@linux.intel.com wrote:
From: Heikki Krogerus heikki.kroge...@linux.intel.com
It's good to have a quasistatic name for the platform driver.
Signed-off-by: Heikki Krogerus heikki.kroge...@linux.intel.com
Signed-off-by: Andy Shevchenko andriy.shevche...@linux.intel.com
---
drivers/dma/dw_dmac.c |1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/dma/dw_dmac.c b/drivers/dma/dw_dmac.c
index 0b88ced..bbb2a82 100644
--- a/drivers/dma/dw_dmac.c
+++ b/drivers/dma/dw_dmac.c
@@ -1712,6 +1712,7 @@ static struct platform_driver dw_driver = {
module_platform_driver(dw_driver);
+MODULE_ALIAS(platform:dw_dmac);
Acked-by: Viresh Kumar viresh.ku...@linaro.org
Reviewed-by: Felipe Balbi ba...@ti.com
--
balbi
signature.asc
Description: Digital signature
Re: [PATCHv3 3/7] dmaengine: dw_dmac: remove CLK dependency
On Thu, Sep 27, 2012 at 01:47:58PM +0530, viresh kumar wrote: On Thu, Sep 27, 2012 at 1:12 PM, Felipe Balbi ba...@ti.com wrote: Hi, On Thu, Sep 27, 2012 at 10:31:57AM +0300, Andy Shevchenko wrote: From: Heikki Krogerus heikki.kroge...@linux.intel.com This driver could be used on different platforms. Thus, the HAVE_CLK dependency is dropped away. Signed-off-by: Heikki Krogerus heikki.kroge...@linux.intel.com Signed-off-by: Andy Shevchenko andriy.shevche...@linux.intel.com --- drivers/dma/Kconfig |1 - 1 file changed, 1 deletion(-) diff --git a/drivers/dma/Kconfig b/drivers/dma/Kconfig index 677cd6e..df32537 100644 --- a/drivers/dma/Kconfig +++ b/drivers/dma/Kconfig @@ -83,7 +83,6 @@ config INTEL_IOP_ADMA config DW_DMAC tristate Synopsys DesignWare AHB DMA support - depends on HAVE_CLK as is, this will break compilation of any arch which doesn't set HAVE_CLK. No, it will not due to following commit id: commit 93abe8e4b13ae9a0428ce940a8a03ac72a7626f1 Author: Viresh Kumar viresh.ku...@st.com Date: Mon Jul 30 14:39:27 2012 -0700 clk: add non CONFIG_HAVE_CLK routines @Andy: Acked-by: Viresh Kumar viresh.ku...@linaro.org Reviewed-by: Felipe Balbi ba...@ti.com -- balbi signature.asc Description: Digital signature
Re: [PATCHv3 6/7] dma: move dw_dmac driver to an own directory
On Thu, Sep 27, 2012 at 01:52:24PM +0530, viresh kumar wrote: On Thu, Sep 27, 2012 at 1:02 PM, Andy Shevchenko andriy.shevche...@linux.intel.com wrote: The dw_dmac driver contains multiple files. To make a managment of them more convenient move it to an own directory. Looks readable now :) Acked-by: Viresh Kumar viresh.ku...@linaro.org (After fixing Felipe's comment) And here's mine (after fixing the typo): Reviewed-by: Felipe Balbi ba...@ti.com -- balbi signature.asc Description: Digital signature
Re: [PATCH] usb: gadget: ncm: correct endianess conversion
On Fri, Oct 05, 2012 at 01:58:21AM +0300, Dmytro Milinevskyy wrote: Signed-off-by: Dmytro Milinevskyy milinevs...@gmail.com missing commit log. NAK. Just resend with a proper commit log and I can apply. -- balbi signature.asc Description: Digital signature
Re: [PATCH] usb: gadget: ncm: correct endianess conversion
On Sun, Nov 04, 2012 at 03:30:21PM +0100, Dmytro Milinevskyy wrote:
Convert USB descriptor's fields to CPU byte order before using locally in USB
NCM gadget driver.
Tested on MIPS32 big-endian device.
Signed-off-by: Dmytro Milinevskyy milinevs...@gmail.com
doesn't apply:
$ patch -p1 --dry-run patch.diff
patching file drivers/usb/gadget/f_ncm.c
patch: malformed patch at line 31: @@ -869,15 +869,19 @@ static struct
sk_buff *ncm_wrap_ntb(struct gether *port,
can you use git-send-email ?
---
drivers/usb/gadget/f_ncm.c | 12
1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/drivers/usb/gadget/f_ncm.c b/drivers/usb/gadget/f_ncm.c
index b651b52..a7cdd47 100644
--- a/drivers/usb/gadget/f_ncm.c
+++ b/drivers/usb/gadget/f_ncm.c
@@ -102,7 +102,7 @@ static inline unsigned ncm_bitrate(struct usb_gadget *g)
USB_CDC_NCM_NTB32_SUPPORTED)
static struct usb_cdc_ncm_ntb_parameters ntb_parameters = {
- .wLength = sizeof ntb_parameters,
+ .wLength = cpu_to_le16(sizeof(ntb_parameters)),
.bmNtbFormatsSupported = cpu_to_le16(FORMATS_SUPPORTED),
.dwNtbInMaxSize = cpu_to_le32(NTB_DEFAULT_IN_SIZE),
.wNdpInDivisor = cpu_to_le16(4),
@@ -869,15 +869,19 @@ static struct sk_buff *ncm_wrap_ntb(struct gether *port,
struct sk_buff *skb2;
int ncb_len = 0;
__le16 *tmp;
- int div = ntb_parameters.wNdpInDivisor;
- int rem = ntb_parameters.wNdpInPayloadRemainder;
+ int div;
+ int rem;
int pad;
- int ndp_align = ntb_parameters.wNdpInAlignment;
+ int ndp_align;
int ndp_pad;
unsignedmax_size = ncm-port.fixed_in_len;
struct ndp_parser_opts *opts = ncm-parser_opts;
unsignedcrc_len = ncm-is_crc ? sizeof(uint32_t) : 0;
+ div = le16_to_cpu(ntb_parameters.wNdpInDivisor);
+ rem = le16_to_cpu(ntb_parameters.wNdpInPayloadRemainder);
+ ndp_align = le16_to_cpu(ntb_parameters.wNdpInAlignment);
+
ncb_len += opts-nth_size;
ndp_pad = ALIGN(ncb_len, ndp_align) - ncb_len;
ncb_len += ndp_pad;
--
1.8.0
--
balbi
signature.asc
Description: Digital signature
Re: [PATCH v4 6/7] usb: dwc3-omap: Minor fixes to get dt working
Hi, On Thu, Oct 25, 2012 at 08:24:09PM +0530, kishon wrote: Hi Benoit, On Thursday 25 October 2012 07:11 PM, Benoit Cousson wrote: Hi Kishon, On 10/15/2012 03:27 PM, Kishon Vijay Abraham I wrote: Includes few minor fixes in dwc3-omap like populating the compatible string in a correct way, extracting the utmi-mode property properly and changing the index of get_irq since irq of core is removed from hwmod entry. Also updated the documentation with dwc3-omap device tree binding information. Signed-off-by: Kishon Vijay Abraham I kis...@ti.com --- drivers/usb/dwc3/dwc3-omap.c | 14 ++ 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/drivers/usb/dwc3/dwc3-omap.c b/drivers/usb/dwc3/dwc3-omap.c index b84ddf3..10aad46 100644 --- a/drivers/usb/dwc3/dwc3-omap.c +++ b/drivers/usb/dwc3/dwc3-omap.c @@ -318,11 +318,10 @@ static int __devinit dwc3_omap_probe(struct platform_device *pdev) struct resource *res; struct device *dev = pdev-dev; - int size; int ret = -ENOMEM; int irq; - const u32 *utmi_mode; + u32 utmi_mode; u32 reg; void __iomem*base; @@ -336,13 +335,13 @@ static int __devinit dwc3_omap_probe(struct platform_device *pdev) platform_set_drvdata(pdev, omap); - irq = platform_get_irq(pdev, 1); + irq = platform_get_irq(pdev, 0); Cannot you use the name of the resource to avoid that kind of trick? *name* is mostly used when we have multiple resource of the same type for a single device. Previously we were clubbing wrapper resources and core resources in a single hwmod device, so we had to use different indexing. But with dt we have separated those under two different devices and hence we can always use index as '0'. But if you think we should have *name*, let me know, I can resend this patch :-) since there were no more replies here, i'm assuming Benoit's happy with Kishon's explanation. Will apply this series as is. -- balbi signature.asc Description: Digital signature
Re: [PATCH v4 0/7] usb: dwc3-omap: add dt support
On Mon, Oct 15, 2012 at 06:57:53PM +0530, Kishon Vijay Abraham I wrote:
This patch series adds dt support to dwc3 core and fixes few minor
stuff in dwc3-omap glue to get dwc3 working.
While at that it also uses *of_platform* to create the child device
(dwc3-core) and fixes to use runtime API's to enable clock and write
to SYSCONFIG register.
Changes from v3:
* rebased to latest commit in balbi's tree
* Fixed few typos in the commit log
* Added *extern* keyword for dwc3_omap_mailbox function declaration
Changes from v2:
* Fixed Sergei comment to use of_property_read_u32 insted of of_get_property
Changes from v1:
* made device_for_each_child() as a seperate patch
* made all other minor fixes wrt typos and function renames
This patch series is developed on:
git://git.kernel.org/pub/scm/linux/kernel/git/balbi/usb.git dwc3
please rebase on my dwc3 branch. Because of commit belo, patches 1 and 2
won't apply:
commit 124dafde8f8174caf5cef1c3eaba001657d66f4f
Author: Sebastian Andrzej Siewior bige...@linutronix.de
Date: Mon Oct 29 18:09:53 2012 +0100
usb: dwc3: remove custom unique id handling
The lockless implementation of the unique id is quite impressive (:P)
but dirver's core can handle it, we can remove it and make our code a
little smaller.
Cc: Anton Tikhomirov av.tikhomi...@samsung.com
Signed-off-by: Sebastian Andrzej Siewior bige...@linutronix.de
Signed-off-by: Felipe Balbi ba...@ti.com
diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
index b923183..d8d327a 100644
--- a/drivers/usb/dwc3/core.c
+++ b/drivers/usb/dwc3/core.c
@@ -66,45 +66,6 @@ MODULE_PARM_DESC(maximum_speed, Maximum supported speed.);
/* --
*/
-#define DWC3_DEVS_POSSIBLE 32
-
-static DECLARE_BITMAP(dwc3_devs, DWC3_DEVS_POSSIBLE);
-
-int dwc3_get_device_id(void)
-{
- int id;
-
-again:
- id = find_first_zero_bit(dwc3_devs, DWC3_DEVS_POSSIBLE);
- if (id DWC3_DEVS_POSSIBLE) {
- int old;
-
- old = test_and_set_bit(id, dwc3_devs);
- if (old)
- goto again;
- } else {
- pr_err(dwc3: no space for new device\n);
- id = -ENOMEM;
- }
-
- return id;
-}
-EXPORT_SYMBOL_GPL(dwc3_get_device_id);
-
-void dwc3_put_device_id(int id)
-{
- int ret;
-
- if (id 0)
- return;
-
- ret = test_bit(id, dwc3_devs);
- WARN(!ret, dwc3: ID %d not in use\n, id);
- smp_mb__before_clear_bit();
- clear_bit(id, dwc3_devs);
-}
-EXPORT_SYMBOL_GPL(dwc3_put_device_id);
-
void dwc3_set_mode(struct dwc3 *dwc, u32 mode)
{
u32 reg;
diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
index 243affc..4999563 100644
--- a/drivers/usb/dwc3/core.h
+++ b/drivers/usb/dwc3/core.h
@@ -868,7 +868,4 @@ void dwc3_host_exit(struct dwc3 *dwc);
int dwc3_gadget_init(struct dwc3 *dwc);
void dwc3_gadget_exit(struct dwc3 *dwc);
-extern int dwc3_get_device_id(void);
-extern void dwc3_put_device_id(int id);
-
#endif /* __DRIVERS_USB_DWC3_CORE_H */
diff --git a/drivers/usb/dwc3/dwc3-exynos.c b/drivers/usb/dwc3/dwc3-exynos.c
index ca65978..586f105 100644
--- a/drivers/usb/dwc3/dwc3-exynos.c
+++ b/drivers/usb/dwc3/dwc3-exynos.c
@@ -94,7 +94,6 @@ static int __devinit dwc3_exynos_probe(struct platform_device
*pdev)
struct dwc3_exynos *exynos;
struct clk *clk;
- int devid;
int ret = -ENOMEM;
exynos = kzalloc(sizeof(*exynos), GFP_KERNEL);
@@ -105,20 +104,16 @@ static int __devinit dwc3_exynos_probe(struct
platform_device *pdev)
platform_set_drvdata(pdev, exynos);
- devid = dwc3_get_device_id();
- if (devid 0)
- goto err1;
-
ret = dwc3_exynos_register_phys(exynos);
if (ret) {
dev_err(pdev-dev, couldn't register PHYs\n);
goto err1;
}
- dwc3 = platform_device_alloc(dwc3, devid);
+ dwc3 = platform_device_alloc(dwc3, PLATFORM_DEVID_AUTO);
if (!dwc3) {
dev_err(pdev-dev, couldn't allocate dwc3 device\n);
- goto err2;
+ goto err1;
}
clk = clk_get(pdev-dev, usbdrd30);
@@ -170,8 +165,6 @@ err4:
clk_put(clk);
err3:
platform_device_put(dwc3);
-err2:
- dwc3_put_device_id(devid);
err1:
kfree(exynos);
err0:
@@ -187,8 +180,6 @@ static int __devexit dwc3_exynos_remove(struct
platform_device *pdev)
platform_device_unregister(exynos-usb2_phy);
platform_device_unregister(exynos-usb3_phy);
- dwc3_put_device_id(exynos-dwc3-id);
-
if (pdata pdata-phy_exit)
pdata-phy_exit(pdev, pdata-phy_type);
diff --git a/drivers/usb/dwc3/dwc3-omap.c b/drivers/usb/dwc3/dwc3-omap.c
index ee57a10
Re: [PATCH] usb: gadget: ncm: correct endianess conversion
Hi, (please, never top-post) On Wed, Nov 07, 2012 at 02:14:00PM +0100, Dmytro Milinevskyy wrote: Unfortunately I have some issues with git send-email. I've attached the patch itself .. I'll apply it like that this time, but try to figure out how to send patches properly. We have some very helpful hints on Documentation/email-clients.txt which are hugely underused ;-) -- balbi signature.asc Description: Digital signature
Re: [PATCH] usb: gadget: ncm: correct endianess conversion
Hi, On Thu, Nov 08, 2012 at 08:07:57PM +0100, Dmytro Milinevskyy wrote: On Wed, Nov 07, 2012 at 02:14:00PM +0100, Dmytro Milinevskyy wrote: Unfortunately I have some issues with git send-email. I've attached the patch itself .. I'll apply it like that this time, but try to figure out how to send patches properly. We have some very helpful hints on Documentation/email-clients.txt which are hugely underused ;-) well, I try to follow the rules as much as possible as long as tools work ... =) git send-email has thousands of users and it works fine for them (including myself). Maybe you just misconfigured it ?!? ;-) cheers -- balbi signature.asc Description: Digital signature
Re: [PATCH v3 resend] USB: PHY: Re-organize Tegra USB PHY driver
Hi,
On Fri, Oct 19, 2012 at 04:03:26PM +0530, Venu Byravarasu wrote:
NVIDIA produces several Tegra SoCs viz Tegra20, Tegra30 etc.
In order to support USB PHY drivers on these SoCs, existing
PHY driver is split into SoC agnostic common USB PHY driver
and Tegra20-specific USB phy driver. This will facilitate
easy addition and deletion of phy drivers for Tegra SoCs.
Signed-off-by: Venu Byravarasu vbyravarasu@xx
you mail ID is wrong here...
---
delta from v3:
Rebased the v3 changes on top of xceiv branch.
delta from v2:
Added an if condition to check for device_node to be not NULL,
before dereferencing it.
---
drivers/usb/host/ehci-tegra.c | 20 +-
drivers/usb/phy/Makefile |1 +
.../usb/phy/{tegra_usb_phy.c = tegra2_usb_phy.c} | 372 ++-
drivers/usb/phy/tegra2_usb_phy.h | 178 +
drivers/usb/phy/tegra_usb_phy.c| 725
+---
include/linux/usb/tegra_usb_phy.h | 34 +-
6 files changed, 304 insertions(+), 1026 deletions(-)
copy drivers/usb/phy/{tegra_usb_phy.c = tegra2_usb_phy.c} (57%)
create mode 100644 drivers/usb/phy/tegra2_usb_phy.h
diff --git a/drivers/usb/host/ehci-tegra.c b/drivers/usb/host/ehci-tegra.c
index 6223d17..e08aea3 100644
--- a/drivers/usb/host/ehci-tegra.c
+++ b/drivers/usb/host/ehci-tegra.c
@@ -618,6 +618,9 @@ static int tegra_ehci_probe(struct platform_device *pdev)
int err = 0;
int irq;
int instance = pdev-id;
+ struct device_node *np = pdev-dev.of_node;
+ struct phy_params params;
+ int phy_type;
pdata = pdev-dev.platform_data;
if (!pdata) {
@@ -706,9 +709,22 @@ static int tegra_ehci_probe(struct platform_device *pdev)
}
}
+ phy_type = of_property_match_string(np, phy_type, utmi);
+ if (phy_type = 0)
+ params.type = TEGRA_USB_PHY_TYPE_UTMI;
+ else {
+ phy_type = of_property_match_string(np, phy_type, ulpi);
+ if (phy_type = 0)
+ params.type = TEGRA_USB_PHY_TYPE_ULPI;
+ else
+ params.type = TEGRA_USB_PHY_TYPE_INVALID;
+ }
+
+ params.mode = TEGRA_USB_PHY_MODE_HOST;
+ params.config = pdata-phy_config;
+
tegra-phy = tegra_usb_phy_open(pdev-dev, instance, hcd-regs,
- pdata-phy_config,
- TEGRA_USB_PHY_MODE_HOST);
+ params);
if (IS_ERR(tegra-phy)) {
dev_err(pdev-dev, Failed to open USB phy\n);
err = -ENXIO;
Alan, if you're ok with the ehci-tegra changes, I can carry this patch
by myself.
diff --git a/drivers/usb/phy/Makefile b/drivers/usb/phy/Makefile
index b069f29..21872e1 100644
--- a/drivers/usb/phy/Makefile
+++ b/drivers/usb/phy/Makefile
@@ -8,3 +8,4 @@ obj-$(CONFIG_OMAP_USB2) += omap-usb2.o
obj-$(CONFIG_USB_ISP1301)+= isp1301.o
obj-$(CONFIG_MV_U3D_PHY) += mv_u3d_phy.o
obj-$(CONFIG_USB_EHCI_TEGRA) += tegra_usb_phy.o
+obj-$(CONFIG_USB_EHCI_TEGRA) += tegra2_usb_phy.o
diff --git a/drivers/usb/phy/tegra_usb_phy.c
b/drivers/usb/phy/tegra2_usb_phy.c
similarity index 57%
copy from drivers/usb/phy/tegra_usb_phy.c
copy to drivers/usb/phy/tegra2_usb_phy.c
index 9d13c81..2ff6dcb 100644
--- a/drivers/usb/phy/tegra_usb_phy.c
+++ b/drivers/usb/phy/tegra2_usb_phy.c
@@ -1,9 +1,11 @@
/*
* Copyright (C) 2010 Google, Inc.
+ * Copyright (c) 2012, NVIDIA CORPORATION. All rights reserved.
*
* Author:
* Erik Gilling konk...@google.com
* Benoit Goby ben...@android.com
+ * Venu Byravarasu vbyravar...@nvidia.com
*
* This software is licensed under the terms of the GNU General Public
* License version 2, as published by the Free Software Foundation, and
@@ -29,191 +31,20 @@
#include linux/usb/ulpi.h
#include asm/mach-types.h
#include linux/usb/tegra_usb_phy.h
+#include mach/iomap.h
-#define TEGRA_USB_BASE 0xC500
-#define TEGRA_USB_SIZE SZ_16K
-
-#define ULPI_VIEWPORT0x170
-
-#define USB_PORTSC1 0x184
-#define USB_PORTSC1_PTS(x) (((x) 0x3) 30)
-#define USB_PORTSC1_PSPD(x)(((x) 0x3) 26)
-#define USB_PORTSC1_PHCD (1 23)
-#define USB_PORTSC1_WKOC (1 22)
-#define USB_PORTSC1_WKDS (1 21)
-#define USB_PORTSC1_WKCN (1 20)
-#define USB_PORTSC1_PTC(x) (((x) 0xf) 16)
-#define USB_PORTSC1_PP (1 12)
-#define USB_PORTSC1_SUSP (1 7)
-#define USB_PORTSC1_PE (1 2)
-#define USB_PORTSC1_CCS(1 0)
-
-#define USB_SUSP_CTRL0x400
-#define USB_WAKE_ON_CNNT_EN_DEV(1 3)
-#define USB_WAKE_ON_DISCON_EN_DEV (1 4)
-#define USB_SUSP_CLR (1 5)
-#define USB_PHY_CLK_VALID (1 7)
-#define UTMIP_RESET(1 11)
Re: [PATCH v3 resend] USB: PHY: Re-organize Tegra USB PHY driver
Hi, On Fri, Oct 19, 2012 at 04:08:05PM +0530, Venu Byravarasu wrote: NVIDIA produces several Tegra SoCs viz Tegra20, Tegra30 etc. In order to support USB PHY drivers on these SoCs, existing PHY driver is split into SoC agnostic common USB PHY driver and Tegra20-specific USB phy driver. This will facilitate easy addition and deletion of phy drivers for Tegra SoCs. Signed-off-by: Venu Byravarasu vbyravar...@nvidia.com I was reading this driver more closely and I have a bunch of questions about it, but the most important of all of them is: why isn't that a real PHY driver ?. It doesn't have a probe() function, it doesn't use struct usb_phy to represent the PHY, it has a bunch of tegra-specific APIs and we can't let those continue. Please, take a look at drivers/usb/phy/omap_usb2.c (misnamed actually, should be phy-omap-usb2.c so we have a common prefix) to see how your PHY driver should look like and which sort of functionality if should expose to the rest of the kernel. Please comment on the above. cheers -- balbi signature.asc Description: Digital signature
Re: [PATCH 1/1] usb: gadget: Don't attempt to dequeue requests for a disabled USB endpoint on Freescale hardware
Hi, On Fri, Oct 19, 2012 at 06:19:26PM +0100, Simon Haggett wrote: Some gadget drivers may attempt to dequeue requests for an endpoint that has already been disabled. For example, in the UVC gadget driver, uvc_function_set_alt() will call usb_ep_disable() when alt setting 0 is selected. When the userspace application subsequently issues the VIDIOC_STREAMOFF ioctl, uvc_video_enable() invokes usb_ep_dequeue() to ensure that all requests have been cancelled. bug is on uvc gadget, then. Laurent ? Also, fsl should be removed from the tree, I'm trying to persuade iMX folks to use drivers/usb/chipidea instead. For the Freescale High Speed Dual-Role USB controller, fsl_ep_dequeue() provides the implementation of usb_ep_dequeue(). If this is called for a disabled endpoint, a kernel oops will occur when the ep-ep.desc field is dereferenced (by ep_index()). fsl_ep_disable() sets this field to NULL, as well as deleting all pending requests for the endpoint. This patch adds an additional check to fsl_ep_dequeue() to ensure that the endpoint has not already been disabled before attempting to dequeue requests. Signed-off-by: Simon Haggett simon.hagg...@realvnc.com --- drivers/usb/gadget/fsl_udc_core.c |5 - 1 files changed, 4 insertions(+), 1 deletions(-) diff --git a/drivers/usb/gadget/fsl_udc_core.c b/drivers/usb/gadget/fsl_udc_core.c index 6ae70cb..acd513b 100644 --- a/drivers/usb/gadget/fsl_udc_core.c +++ b/drivers/usb/gadget/fsl_udc_core.c @@ -955,7 +955,10 @@ static int fsl_ep_dequeue(struct usb_ep *_ep, struct usb_request *_req) int ep_num, stopped, ret = 0; u32 epctrl; - if (!_ep || !_req) + /* Ensure that the ep and request are valid, and the ep is not + * disabled + */ + if (!_ep || !_req || !ep-ep.desc) return -EINVAL; spin_lock_irqsave(ep-udc-lock, flags); -- 1.7.4.1 -- balbi signature.asc Description: Digital signature
Re: [PATCH] i2c: omap: adopt pinctrl support
Hi,
On Tue, Oct 16, 2012 at 05:23:20PM +0200, Sebastien Guiriec wrote:
Some GPIO expanders need some early pin control muxing. Due to
legacy boards sometimes the driver uses subsys_initcall instead of
module_init. This patch takes advantage of defer probe feature
and pin control in order to wait until pin control probing before
GPIO driver probing. It has been tested on OMAP5 board with TCA6424
driver.
log:
[0.482299] omap_i2c i2c.15: could not find pctldev for node /ocp/pinmux@4a00
2840/pinmux_i2c5_pins, deferring probe
[0.482330] platform i2c.15: Driver omap_i2c requests probe deferral
[0.484466] Advanced Linux Sound Architecture Driver Initialized.
[4.746917] omap_i2c i2c.15: bus 4 rev2.4.0 at 100 kHz
[4.755279] gpiochip_find_base: found new base at 477
[4.761169] gpiochip_add: registered GPIOs 477 to 500 on device: tca6424a
Signed-off-by: Sebastien Guiriec s-guir...@ti.com
looks good to me also, should go in v3.8 merge window:
Reviewed-by: Felipe Balbi ba...@ti.com
---
drivers/i2c/busses/i2c-omap.c | 13 +
1 file changed, 13 insertions(+)
diff --git a/drivers/i2c/busses/i2c-omap.c b/drivers/i2c/busses/i2c-omap.c
index db31eae..661d8a2 100644
--- a/drivers/i2c/busses/i2c-omap.c
+++ b/drivers/i2c/busses/i2c-omap.c
@@ -44,6 +44,7 @@
#include linux/i2c-omap.h
#include linux/pm_runtime.h
#include linux/pm_qos.h
+#include linux/pinctrl/consumer.h
/* I2C controller revisions */
#define OMAP_I2C_OMAP1_REV_2 0x20
@@ -213,6 +214,8 @@ struct omap_i2c_dev {
u16 syscstate;
u16 westate;
u16 errata;
+
+ struct pinctrl *pins;
};
static const u8 reg_map_ip_v1[] = {
@@ -1107,6 +1110,16 @@ omap_i2c_probe(struct platform_device *pdev)
dev-dtrev = pdata-rev;
}
+ dev-pins = devm_pinctrl_get_select_default(pdev-dev);
+ if (IS_ERR(dev-pins)) {
+ if (PTR_ERR(dev-pins) == -EPROBE_DEFER)
+ return -EPROBE_DEFER;
+
+ dev_warn(pdev-dev, did not get pins for i2c error: %li\n,
+ PTR_ERR(dev-pins));
+ dev-pins = NULL;
+ }
+
dev-dev = pdev-dev;
dev-irq = irq;
--
1.7.10.4
--
balbi
signature.asc
Description: Digital signature
Re: [PATCH 1/1] usb: gadget: Don't attempt to dequeue requests for a disabled USB endpoint on Freescale hardware
On Mon, Oct 22, 2012 at 03:33:19AM +, Li Yang-R58472 wrote: -Original Message- From: Felipe Balbi [mailto:ba...@ti.com] Sent: Saturday, October 20, 2012 1:37 AM To: Simon Haggett Cc: Li Yang-R58472; Felipe Balbi; Greg Kroah-Hartman; linux- u...@vger.kernel.org; linuxppc-...@lists.ozlabs.org; linux- ker...@vger.kernel.org; Laurent Pinchart Subject: Re: [PATCH 1/1] usb: gadget: Don't attempt to dequeue requests for a disabled USB endpoint on Freescale hardware Hi, On Fri, Oct 19, 2012 at 06:19:26PM +0100, Simon Haggett wrote: Some gadget drivers may attempt to dequeue requests for an endpoint that has already been disabled. For example, in the UVC gadget driver, uvc_function_set_alt() will call usb_ep_disable() when alt setting 0 is selected. When the userspace application subsequently issues the VIDIOC_STREAMOFF ioctl, uvc_video_enable() invokes usb_ep_dequeue() to ensure that all requests have been cancelled. bug is on uvc gadget, then. Laurent ? Also, fsl should be removed from the tree, I'm trying to persuade iMX folks to use drivers/usb/chipidea instead. Besides the iMX usage, the driver is also being used by many Freescale PowerPC/Coldfire SoCs. I agree that it's ideal to move to a common driver. But it is a large task to make the chipidea driver works for all the hardware that fsl_udc had supported and been tested on. I understand that, but we just can't keep so many duplicated drivers in mainline. chipidea udc had at least 3 different implementations. Now it's the time to combine all of those and stick to a single driver. Just make a plan to slowly move towards chipidea in the upcoming few merge windows. I can continue to take in bugfixes for fsl_udc, but only if I see that you guys are working towards merging with chipidea driver. -- balbi signature.asc Description: Digital signature
Re: [PATCH v3 resend] USB: PHY: Re-organize Tegra USB PHY driver
Hi, On Fri, Oct 19, 2012 at 10:29:35AM -0600, Stephen Warren wrote: On 10/19/2012 09:35 AM, Felipe Balbi wrote: Hi, On Fri, Oct 19, 2012 at 04:08:05PM +0530, Venu Byravarasu wrote: NVIDIA produces several Tegra SoCs viz Tegra20, Tegra30 etc. In order to support USB PHY drivers on these SoCs, existing PHY driver is split into SoC agnostic common USB PHY driver and Tegra20-specific USB phy driver. This will facilitate easy addition and deletion of phy drivers for Tegra SoCs. Signed-off-by: Venu Byravarasu vbyravar...@nvidia.com I was reading this driver more closely and I have a bunch of questions about it, but the most important of all of them is: why isn't that a real PHY driver ?. It doesn't have a probe() function, it doesn't use struct usb_phy to represent the PHY, it has a bunch of tegra-specific APIs and we can't let those continue. One question here: If the PHY driver API changes, there will need to be a bunch of ehci-tegra.c changes too. Will you take all those hmm.. indeed. through the PHY tree? If you expect to do that, then I'd like to I can take those if Alan is ok with it :-) Alan ? request you also take: usb: host: tegra remove include of mach/iomap.h http://www.spinics.net/lists/linux-usb/msg72429.html ... since that should get merged before any large changes to ehci-tegra.c; it's the EHCI equivalent of the PHY patch you already merged. (The same request applies to put that into a branch I can pull into the Tegra tree as a basis for cleanup in the Tegra tree) sure, that should be simple enough to do ;-) -- balbi signature.asc Description: Digital signature
Re: [PATCH v3 resend] USB: PHY: Re-organize Tegra USB PHY driver
Hi, On Mon, Oct 22, 2012 at 02:00:00PM +0530, Venu Byravarasu wrote: -Original Message- From: Felipe Balbi [mailto:ba...@ti.com] Sent: Friday, October 19, 2012 9:06 PM To: Venu Byravarasu Cc: st...@rowland.harvard.edu; gre...@linuxfoundation.org; ba...@ti.com; linux-...@vger.kernel.org; linux-kernel@vger.kernel.org Subject: Re: [PATCH v3 resend] USB: PHY: Re-organize Tegra USB PHY driver * PGP Signed by an unknown key Hi, On Fri, Oct 19, 2012 at 04:08:05PM +0530, Venu Byravarasu wrote: NVIDIA produces several Tegra SoCs viz Tegra20, Tegra30 etc. I was reading this driver more closely and I have a bunch of questions about it, but the most important of all of them is: why isn't that a real PHY driver ?. It doesn't have a probe() function, it doesn't use struct usb_phy to represent the PHY, it has a bunch of tegra-specific APIs and we can't let those continue. Please, take a look at drivers/usb/phy/omap_usb2.c (misnamed actually, should be phy-omap-usb2.c so we have a common prefix) to see how your PHY driver should look like and which sort of functionality if should expose to the rest of the kernel. Hi Felipe, I'll go through omap phy driver and prepare similar patches for tegra phy driver and push them with upcoming patches. As current patch is mostly re-organizing the existing phy driver, can you plz merge This as is? I would have to convince me about the need for that (and I'm open to be convinced ;-), because if a later series of patches will come getting rid of the current driver and turning it into a real PHY driver, I don't see the benefit of taking $SUBJECT. -- balbi signature.asc Description: Digital signature
Re: [PATCH v3 resend] USB: PHY: Re-organize Tegra USB PHY driver
Hi, On Mon, Oct 22, 2012 at 03:47:02PM +0530, Venu Byravarasu wrote: -Original Message- From: Felipe Balbi [mailto:ba...@ti.com] Sent: Monday, October 22, 2012 3:33 PM To: Venu Byravarasu Cc: ba...@ti.com; st...@rowland.harvard.edu; gre...@linuxfoundation.org; linux-...@vger.kernel.org; linux- ker...@vger.kernel.org Subject: Re: [PATCH v3 resend] USB: PHY: Re-organize Tegra USB PHY driver * PGP Signed by an unknown key Hi, On Mon, Oct 22, 2012 at 02:00:00PM +0530, Venu Byravarasu wrote: -Original Message- From: Felipe Balbi [mailto:ba...@ti.com] Sent: Friday, October 19, 2012 9:06 PM To: Venu Byravarasu Cc: st...@rowland.harvard.edu; gre...@linuxfoundation.org; ba...@ti.com; linux-...@vger.kernel.org; linux-kernel@vger.kernel.org Subject: Re: [PATCH v3 resend] USB: PHY: Re-organize Tegra USB PHY driver Old Signed by an unknown key Hi, On Fri, Oct 19, 2012 at 04:08:05PM +0530, Venu Byravarasu wrote: NVIDIA produces several Tegra SoCs viz Tegra20, Tegra30 etc. I was reading this driver more closely and I have a bunch of questions about it, but the most important of all of them is: why isn't that a real PHY driver ?. It doesn't have a probe() function, it doesn't use struct usb_phy to represent the PHY, it has a bunch of tegra-specific APIs and we can't let those continue. Please, take a look at drivers/usb/phy/omap_usb2.c (misnamed actually, should be phy-omap-usb2.c so we have a common prefix) to see how your PHY driver should look like and which sort of functionality if should expose to the rest of the kernel. Hi Felipe, I'll go through omap phy driver and prepare similar patches for tegra phy driver and push them with upcoming patches. As current patch is mostly re-organizing the existing phy driver, can you plz merge This as is? I would have to convince me about the need for that (and I'm open to be convinced ;-), because if a later series of patches will come getting rid of the current driver and turning it into a real PHY driver, I don't see the benefit of taking $SUBJECT. Hi Felipe, The current patch splits out the existing tegra USB phy driver into two parts, as you would have already noticed from the code. The probe and etc changes that you asked to add, will be applicable to common Phy driver and should not have any implications on SOC dependent phy driver. what is this SOC dependent PHY driver ? What sort of dependencies are there ? Those differences should be handled with runtime checks. -- balbi signature.asc Description: Digital signature
Re: [PATCH 1/1] usb: gadget: Don't attempt to dequeue requests for a disabled USB endpoint on Freescale hardware
Hi, On Mon, Oct 22, 2012 at 12:47:21PM +0200, Laurent Pinchart wrote: Hi, On Monday 22 October 2012 03:33:19 Li Yang-R58472 wrote: On Saturday, October 20, 2012 1:37 AM Felipe Balbi wrote: On Fri, Oct 19, 2012 at 06:19:26PM +0100, Simon Haggett wrote: Some gadget drivers may attempt to dequeue requests for an endpoint that has already been disabled. For example, in the UVC gadget driver, uvc_function_set_alt() will call usb_ep_disable() when alt setting 0 is selected. When the userspace application subsequently issues the VIDIOC_STREAMOFF ioctl, uvc_video_enable() invokes usb_ep_dequeue() to ensure that all requests have been cancelled. bug is on uvc gadget, then. Laurent ? We've discussed this topic a couple of months before. I believe that's not a bug. http://68.183.106.108/lists/linux-usb/msg68869.html fair enough :-) That's a different case, however. At the link above we're discussing dequeueing a request which is already being dequeued. $SUBJECT is trying to fix dequeueing of a request for an endpoint which isn't even enabled. -- balbi signature.asc Description: Digital signature
[PATCH] arm: sched: stop sched_clock() during suspend
The scheduler imposes a requirement to sched_clock()
which is to stop the clock during suspend, if we don't
do that IRQ threads will be rescheduled in the future
which might cause transfers to timeout depending on
how driver is written.
This became an issue on OMAP when we converted omap-i2c.c
to use threaded IRQs, it turned out that depending on how
much time we spent on suspend, the I2C IRQ thread would
end up being rescheduled so far in the future that I2C
transfers would timeout and, because omap_hsmmc depends
on an I2C-connected device to detect if an MMC card is
inserted in the slot, our rootfs would just vanish.
arch/arm/kernel/sched_clock.c already had an optional
implementation (sched_clock_needs_suspend()) which would
handle scheduler's requirement properly, what this patch
does is simply to make that implementation non-optional.
This has been tested with beagleboard XM (OMAP3630) and
pandaboard rev A3 (OMAP4430). Suspend to RAM is now working
after this patch.
Signed-off-by: Felipe Balbi ba...@ti.com
---
arch/arm/include/asm/sched_clock.h | 2 --
arch/arm/kernel/sched_clock.c | 18 --
2 files changed, 4 insertions(+), 16 deletions(-)
diff --git a/arch/arm/include/asm/sched_clock.h
b/arch/arm/include/asm/sched_clock.h
index 05b8e82..e3f7572 100644
--- a/arch/arm/include/asm/sched_clock.h
+++ b/arch/arm/include/asm/sched_clock.h
@@ -10,7 +10,5 @@
extern void sched_clock_postinit(void);
extern void setup_sched_clock(u32 (*read)(void), int bits, unsigned long rate);
-extern void setup_sched_clock_needs_suspend(u32 (*read)(void), int bits,
- unsigned long rate);
#endif
diff --git a/arch/arm/kernel/sched_clock.c b/arch/arm/kernel/sched_clock.c
index e21bac2..fc6692e 100644
--- a/arch/arm/kernel/sched_clock.c
+++ b/arch/arm/kernel/sched_clock.c
@@ -107,13 +107,6 @@ static void sched_clock_poll(unsigned long wrap_ticks)
update_sched_clock();
}
-void __init setup_sched_clock_needs_suspend(u32 (*read)(void), int bits,
- unsigned long rate)
-{
- setup_sched_clock(read, bits, rate);
- cd.needs_suspend = true;
-}
-
void __init setup_sched_clock(u32 (*read)(void), int bits, unsigned long rate)
{
unsigned long r, w;
@@ -189,18 +182,15 @@ void __init sched_clock_postinit(void)
static int sched_clock_suspend(void)
{
sched_clock_poll(sched_clock_timer.data);
- if (cd.needs_suspend)
- cd.suspended = true;
+ cd.suspended = true;
return 0;
}
static void sched_clock_resume(void)
{
- if (cd.needs_suspend) {
- cd.epoch_cyc = read_sched_clock();
- cd.epoch_cyc_copy = cd.epoch_cyc;
- cd.suspended = false;
- }
+ cd.epoch_cyc = read_sched_clock();
+ cd.epoch_cyc_copy = cd.epoch_cyc;
+ cd.suspended = false;
}
static struct syscore_ops sched_clock_ops = {
--
1.8.0.rc0
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] arm: sched: stop sched_clock() during suspend
Hi,
On Mon, Oct 22, 2012 at 02:54:37PM +0300, Felipe Balbi wrote:
The scheduler imposes a requirement to sched_clock()
which is to stop the clock during suspend, if we don't
do that IRQ threads will be rescheduled in the future
which might cause transfers to timeout depending on
how driver is written.
This became an issue on OMAP when we converted omap-i2c.c
to use threaded IRQs, it turned out that depending on how
much time we spent on suspend, the I2C IRQ thread would
end up being rescheduled so far in the future that I2C
transfers would timeout and, because omap_hsmmc depends
on an I2C-connected device to detect if an MMC card is
inserted in the slot, our rootfs would just vanish.
arch/arm/kernel/sched_clock.c already had an optional
implementation (sched_clock_needs_suspend()) which would
handle scheduler's requirement properly, what this patch
does is simply to make that implementation non-optional.
This has been tested with beagleboard XM (OMAP3630) and
pandaboard rev A3 (OMAP4430). Suspend to RAM is now working
after this patch.
Signed-off-by: Felipe Balbi ba...@ti.com
just adding more guys to Cc. Please add more if I have missed someone.
---
arch/arm/include/asm/sched_clock.h | 2 --
arch/arm/kernel/sched_clock.c | 18 --
2 files changed, 4 insertions(+), 16 deletions(-)
diff --git a/arch/arm/include/asm/sched_clock.h
b/arch/arm/include/asm/sched_clock.h
index 05b8e82..e3f7572 100644
--- a/arch/arm/include/asm/sched_clock.h
+++ b/arch/arm/include/asm/sched_clock.h
@@ -10,7 +10,5 @@
extern void sched_clock_postinit(void);
extern void setup_sched_clock(u32 (*read)(void), int bits, unsigned long
rate);
-extern void setup_sched_clock_needs_suspend(u32 (*read)(void), int bits,
- unsigned long rate);
#endif
diff --git a/arch/arm/kernel/sched_clock.c b/arch/arm/kernel/sched_clock.c
index e21bac2..fc6692e 100644
--- a/arch/arm/kernel/sched_clock.c
+++ b/arch/arm/kernel/sched_clock.c
@@ -107,13 +107,6 @@ static void sched_clock_poll(unsigned long wrap_ticks)
update_sched_clock();
}
-void __init setup_sched_clock_needs_suspend(u32 (*read)(void), int bits,
- unsigned long rate)
-{
- setup_sched_clock(read, bits, rate);
- cd.needs_suspend = true;
-}
-
void __init setup_sched_clock(u32 (*read)(void), int bits, unsigned long
rate)
{
unsigned long r, w;
@@ -189,18 +182,15 @@ void __init sched_clock_postinit(void)
static int sched_clock_suspend(void)
{
sched_clock_poll(sched_clock_timer.data);
- if (cd.needs_suspend)
- cd.suspended = true;
+ cd.suspended = true;
return 0;
}
static void sched_clock_resume(void)
{
- if (cd.needs_suspend) {
- cd.epoch_cyc = read_sched_clock();
- cd.epoch_cyc_copy = cd.epoch_cyc;
- cd.suspended = false;
- }
+ cd.epoch_cyc = read_sched_clock();
+ cd.epoch_cyc_copy = cd.epoch_cyc;
+ cd.suspended = false;
}
static struct syscore_ops sched_clock_ops = {
--
1.8.0.rc0
--
balbi
signature.asc
Description: Digital signature
Re: [PATCHv2] Input: omap4-keypad: Add pinctrl support
Hi, On Tue, Oct 23, 2012 at 12:04:01PM +0200, Linus Walleij wrote: On Tue, Oct 23, 2012 at 11:35 AM, Benoit Cousson b-cous...@ti.com wrote: On 10/23/2012 11:13 AM, Linus Walleij wrote: So Sourav, please tell us a bit about your plans for this and other drivers! Yeah, this idea is to handle pinctrl from all the drivers, and potentially change the mode during suspend when it is relevant. I'm leaning toward the same approach for ux500. But it appears that shmobile prefer to get all resources using bus notifiers. So we need to form some kind of consensus ... or live with the fact that different systems do it different ways. Which will explode the day we need to use a driver on two systems, each using the other approach :-) I much prefer having drivers explicitly manage all their resources, which would mean that pinctrl calls need to be done on probe() and, if necessary, during suspend()/resume(). Using bus notifiers for that is quite a hack IMHO. -- balbi signature.asc Description: Digital signature
Re: [PATCHv2] Input: omap4-keypad: Add pinctrl support
Hi, On Tue, Oct 23, 2012 at 12:29:28PM +0200, Linus Walleij wrote: On Tue, Oct 23, 2012 at 12:23 PM, Thomas Petazzoni thomas.petazz...@free-electrons.com wrote: On Tue, 23 Oct 2012 13:03:33 +0300, Felipe Balbi wrote: But it appears that shmobile prefer to get all resources using bus notifiers. So we need to form some kind of consensus ... or live with the fact that different systems do it different ways. Which will explode the day we need to use a driver on two systems, each using the other approach :-) I much prefer having drivers explicitly manage all their resources, which would mean that pinctrl calls need to be done on probe() and, if necessary, during suspend()/resume(). Using bus notifiers for that is quite a hack IMHO. Agreed. Just like drivers do their ioremap, request_irq and others, they should also request their pin resources using the pinctrl API. Hiding this behind a bus notifier is not nice. So the biggest implementation of the notifier approach to resource handling is the SH clock thing: drivers/base/power/clock_ops.c that's different right ? It's just creating the list of clocks, device drivers still have to call pm_clk_add(). That's ok, I guess, otherwise all struct device would allocate memory which hardly ever used (so far). -- balbi signature.asc Description: Digital signature
Re: [PATCHv2] Input: omap4-keypad: Add pinctrl support
Hi, On Tue, Oct 23, 2012 at 12:45:33PM +0200, Linus Walleij wrote: On Tue, Oct 23, 2012 at 12:29 PM, Felipe Balbi ba...@ti.com wrote: On Tue, Oct 23, 2012 at 12:29:28PM +0200, Linus Walleij wrote: So the biggest implementation of the notifier approach to resource handling is the SH clock thing: drivers/base/power/clock_ops.c that's different right ? It's just creating the list of clocks, device drivers still have to call pm_clk_add(). That's ok, I guess, otherwise all struct device would allocate memory which hardly ever used (so far). Hm so I have had this idea of runtime PM core helping out with pins, so I could add something like pm_pins_fetch() pm_pins_default() pm_pins_idle() pm_pins_sleep() So if one is using the pin states defined in linux/pinctrl/pinctrl-state.h then the PM core can help out in keeping track of the pins and states, and the driver will just tell the PM core what to do and when. Would this fit the bill for everyone's code consolidation needs? It would sure work for us... It however require that no custom states are used and that we keep to the state semantics I just happen to think is most common. From a quick read, it looks ok. I guess problems will only how up when we actually end up with a silicon errata or something similar which mandates that we change pin's state at a particular location. Not sure if we have those yet. -- balbi signature.asc Description: Digital signature
Re: [PATCH v3 resend] USB: PHY: Re-organize Tegra USB PHY driver
Hi, On Tue, Oct 23, 2012 at 04:23:19PM +0530, Venu Byravarasu wrote: -Original Message- From: Venu Byravarasu Sent: Monday, October 22, 2012 4:04 PM To: 'ba...@ti.com' Cc: st...@rowland.harvard.edu; gre...@linuxfoundation.org; linux- u...@vger.kernel.org; linux-kernel@vger.kernel.org Subject: RE: [PATCH v3 resend] USB: PHY: Re-organize Tegra USB PHY driver what is this SOC dependent PHY driver ? SOC dependent PHY driver actually deals with the PHY interface programming. e.g. please see code present in tegra2_usb_driver.c. What sort of dependencies are there ? Those differences should be handled with runtime checks. As PHY related bugs got fixed across different set of SOCs apart from adding few features, wanted to separate this out from common PHY functionality. This will help us in adding support for different SOCs with minimum set of changes. Felipe, Please let me know if you have any more questions on this patch. If not, can you please merge this? Like I said before, those differences should be handled by runtime checks, you shouldn't need separate files for that. What you need to do is implement a single PHY driver which can handle all tegras known to date, later you can add support for new tegras by patching a few things here and there. -- balbi signature.asc Description: Digital signature
Re: [PATCH] usb: remove CONFIG_USB_MUSB_HOST etc
Hi,
On Tue, Oct 23, 2012 at 06:04:53PM +0530, Sekhar Nori wrote:
On 10/8/2012 6:47 PM, Constantine Shulyupin wrote:
From: Constantine Shulyupin co...@makelinux.com
Remove USB configuration in arch/arm/mach-davinci/usb.c accordingly
CONFIG_USB_MUSB_OTG CONFIG_USB_MUSB_PERIPHERAL CONFIG_USB_MUSB_HOST
and set MUSB_OTG configuration by default
because this configuration options are removed from Kconfig.
Signed-off-by: Constantine Shulyupin co...@makelinux.com
Queuing this patch for v3.8. Since the config options are removed there
is no use having code which refers to them. The patch has been tested on
DM644x and DM365 in both host and gadget mode (I will add this
information to commit text while committing). Without this patch .mode
seems to be defaulting to MUSB_UNDEFINED which I think is definitely wrong.
sorry for the delay, this looks ok:
Acked-by: Felipe Balbi ba...@ti.com
Thanks,
Sekhar
---
arch/arm/mach-davinci/usb.c |6 --
1 file changed, 6 deletions(-)
diff --git a/arch/arm/mach-davinci/usb.c b/arch/arm/mach-davinci/usb.c
index f77b953..34509ff 100644
--- a/arch/arm/mach-davinci/usb.c
+++ b/arch/arm/mach-davinci/usb.c
@@ -42,14 +42,8 @@ static struct musb_hdrc_config musb_config = {
};
static struct musb_hdrc_platform_data usb_data = {
-#if defined(CONFIG_USB_MUSB_OTG)
/* OTG requires a Mini-AB connector */
.mode = MUSB_OTG,
-#elif defined(CONFIG_USB_MUSB_PERIPHERAL)
- .mode = MUSB_PERIPHERAL,
-#elif defined(CONFIG_USB_MUSB_HOST)
- .mode = MUSB_HOST,
-#endif
.clock = usb,
.config = musb_config,
};
--
balbi
signature.asc
Description: Digital signature
Re: [PATCHv2] Input: omap4-keypad: Add pinctrl support
HI, On Tue, Oct 23, 2012 at 07:02:09AM -1000, Mitch Bradley wrote: On 10/23/2012 12:03 AM, Felipe Balbi wrote: Hi, I much prefer having drivers explicitly manage all their resources, which would mean that pinctrl calls need to be done on probe() and, if necessary, during suspend()/resume(). Per-driver resource management is certainly convenient when you are dealing with a single system, but it becomes difficult to maintain for drivers that are shared among many platforms. why ? look at drivers/usb/dwc3/, we're using that on OMAP, exynos, PCIe and a couple of different FPGA implementations inside TI. Not to mention what other licensees of that IP core might have internally. So far no problesm with resources at all. We have frameworks exactly to hide the differences. The industry trend for many years has been consolidation around a single programming model per class of device. For example, SDHCI, EHCI, ATA. This trend will only accelerate, as the cost of developing controller IP and associated drivers increases. Such drivers need to be as platform-agnostic as possible. that's why we have pinctrl framework to abstract the details about pin muxing. -- balbi signature.asc Description: Digital signature
Re: [PATCHv2] Input: omap4-keypad: Add pinctrl support
Hi, (please don't top-post) On Tue, Oct 23, 2012 at 07:51:22AM -1000, Mitch Bradley wrote: Perhaps I misunderstood what you were suggesting. I thought that, when you said explicitly manage all their resources, you meant that the driver should know the platform-specific details about clocks and power domains. That is one possible interpretation of the word explicit. we had two suggestions in the thread: 1) handle it in driver source code (explict) 2) handle at bus notifiers level (hidden) archives would've helped you clear up that confusion ;-) -- balbi signature.asc Description: Digital signature
Re: [PATCHv2] Input: omap4-keypad: Add pinctrl support
Hi, On Tue, Oct 23, 2012 at 01:02:49PM -0700, Dmitry Torokhov wrote: On Tue, Oct 23, 2012 at 11:18:12AM +0200, Benoit Cousson wrote: Hi Dimitry, On 10/22/2012 05:50 PM, Dmitry Torokhov wrote: Hi Sourav, On Mon, Oct 22, 2012 at 06:43:00PM +0530, Sourav Poddar wrote: Adapt keypad to use pinctrl framework. Tested on omap4430 sdp with 3.7-rc1 kernel. I do not see anything in the driver that would directly use pinctrl. Is there a better place to select default pin configuration; maybe when instantiating platform device? Why? The devm_pinctrl_get_select_default is using the pinctrl. No, I guess we ihave different understanding of what directly use means. This particular driver does directly use interrupts: it requests it and performs some actions when interrupt arrives. Same goes for IO memory - it gets requested, then we access it. With pinctrl we do not do anything - we just ask another layer to configure it and that is it. this is true for almost anything we do: - we ask another layer to allocate memory for us - we ask another layer to call our ISR once the IRQ line is asserted - we ask another layer to handle the input events we just received - we ask another layer to transfer data through DMA for us - we ask another layer to turn regulators on and off. and so on. This is just how abstractions work, we group common parts in a framework so that users don't need to know the details, but still need to tell the framework when to fiddle with those resources. I have seen just in a few days 3 or 4 drivers having exactly the same change - call to devm_pinctrl_get_select_default(), and I guess I will receive the same patches for the rest of input drivers shortly. This suggests that the operation is done at the wrong level. Do the pin configuration as you parse DT data, the same way you set up i2c devices registers in of_i2c.c, and leave the individual drivers that do not care about specifics alone. Makes no sense to hide that from drivers. The idea here is that driver should know when it needs its pins to muxed correctly. 95% of the time it will be done during probe() but then again, so what ? doing that when parsing DT, or on bus notifiers is just plain wrong. Drivers should be required to handle all of their resources. That's why it is named get_select_default and not get only. This API ensure that the pin will be set to the default state, and this is all we need to do during the probe. Why during the probe and not by default? Realistically, the driver will be loaded as long as there is a matching device and pins will need to be configured. likewise memory will be allocated when matching happens, IRQs will be allocated, regulators will be turned on. So why don't we do all that by default ? Because it is wrong. There is no point to change the mux if the driver is not probed, because potentially the pin can be use be another driver. What other driver would use it? Who would chose what driver to load? Well, you _do_ know that on a SoC we have a limited amount of pins right ? Considering the amont of features which are packed inside a single die, it's not farfetched to assume we will have a lot less pins then we actually need, so we need muxers behind each pin in order to choose which functionality we want. If it happens that keypad's pins are shared with another IP (e.g. GPIO), we need to give the final user (a OEM/ODM) the choice of using those pins as either keypad or GPIOs, thus the need for pinctrl framework and the calls in the drivers. -- balbi signature.asc Description: Digital signature
Re: [PATCH] Revert serial: omap: fix software flow control
Hi Greg, On Tue, Oct 16, 2012 at 08:13:59AM -0700, Tony Lindgren wrote: * Felipe Balbi ba...@ti.com [121016 07:16]: This reverts commit 957ee7270d632245b43f6feb0e70d9a5e9ea6cf6 (serial: omap: fix software flow control). As Russell has pointed out, that commit isn't fixing Software Flow Control at all, and it actually makes it even more broken. It was agreed to revert this commit and use Russell's latest UART patches instead. Cc: Russell King li...@arm.linux.org.uk Signed-off-by: Felipe Balbi ba...@ti.com This seems like the best way to go for the -rc series: Acked-by: Tony Lindgren t...@atomide.com Any chance you can pick this one up for v3.7-rc3 ? cheers -- balbi signature.asc Description: Digital signature
Re: [PATCHv2] Input: omap4-keypad: Add pinctrl support
Hi, On Wed, Oct 24, 2012 at 09:14:29AM -0700, Dmitry Torokhov wrote: snip No, I guess we ihave different understanding of what directly use means. This particular driver does directly use interrupts: it requests it and performs some actions when interrupt arrives. Same goes for IO memory - it gets requested, then we access it. With pinctrl we do not do anything - we just ask another layer to configure it and that is it. this is true for almost anything we do: - we ask another layer to allocate memory for us - we ask another layer to call our ISR once the IRQ line is asserted - we ask another layer to handle the input events we just received - we ask another layer to transfer data through DMA for us - we ask another layer to turn regulators on and off. But we are _directly_ _using_ all of these. You allocate memory and you (the driver) stuff data into that memory. You ask for DMA and you take the DMAed data and work with it. Not so with pinctrl in omap keypad and other drivers I have seen so far. of course we are. If we don't mux the pins to their correct setting, we can't realy use the HW. So while you don't see any SW control of the requested pins, we're still making use of them. and so on. This is just how abstractions work, we group common parts in a framework so that users don't need to know the details, but still need to tell the framework when to fiddle with those resources. I have seen just in a few days 3 or 4 drivers having exactly the same change - call to devm_pinctrl_get_select_default(), and I guess I will receive the same patches for the rest of input drivers shortly. This suggests that the operation is done at the wrong level. Do the pin configuration as you parse DT data, the same way you set up i2c devices registers in of_i2c.c, and leave the individual drivers that do not care about specifics alone. Makes no sense to hide that from drivers. The idea here is that driver should know when it needs its pins to muxed correctly. The driver also needs memory controller to be initialized, gpio chip be ready and registered, DMA subsystem ready, input core reade, etc, etc, etc. You however do not have every driver explicitly initialize any of that; you expect certain working environment to be already operable. The driver does manage resources it controls, it has ultimate knowledge about, pin configuration is normally is not it. We just need to know that we wired/muxed properly, otherwise we won't work. So please let parent layers deal with it. 95% of the time it will be done during probe() but then again, so what ? doing that when parsing DT, or on bus notifiers is just plain wrong. Drivers should be required to handle all of their resources. All of _their_ resources, exactly. They do not own nor control pins so they should not be bothered with them either. Look, when you see that except that they *own* the pins. Now that the muxer has been setup properly, this particular IP owns the pins. potentially _every_ driver in the system needs to set up the same object that it doe snot use otherwise you should realize that individual driver is not the proper place to do that. fair enough, but IMHO, we're not there yet. We can't make that claim yet. Besides, we don't know what's the default pin state in a system. It might be that certain pins start out in a way which consumes less power due to the internal construction of the SoC. If we set pins up before driver probes, and probe fails or the driver is never really used, then we could be falling into a situation where we're wasting power. Granted, you can undo everything you did before, but I guess keeping track of everything we setup before probe() just to remove a couple of lines from drivers is wrong. That's why it is named get_select_default and not get only. This API ensure that the pin will be set to the default state, and this is all we need to do during the probe. Why during the probe and not by default? Realistically, the driver will be loaded as long as there is a matching device and pins will need to be configured. likewise memory will be allocated when matching happens, IRQs will be allocated, regulators will be turned on. So why don't we do all that by default ? Because it is wrong. No, because we do not know how. The generic layer does not know the ISR to install, how much memory to allocate, etc. Having regulator turned on before getting to probe might not be a bad idea. what if your driver never probes ? Will you really leave regulators on consuming extra, valuable power ? There is no point to change the mux if the driver is not probed, because potentially the pin can be use be another driver. What other driver would use it? Who would chose what driver to load? Well, you _do_ know that on a SoC we have a limited amount of pins right ? Considering the amont of
Re: [PATCHv2] Input: omap4-keypad: Add pinctrl support
Hi, On Wed, Oct 24, 2012 at 09:18:01AM -0700, Dmitry Torokhov wrote: On Wed, Oct 24, 2012 at 02:54:23PM +0200, Linus Walleij wrote: On Tue, Oct 23, 2012 at 10:02 PM, Dmitry Torokhov dmitry.torok...@gmail.com wrote: I have seen just in a few days 3 or 4 drivers having exactly the same change - call to devm_pinctrl_get_select_default(), and I guess I will receive the same patches for the rest of input drivers shortly. This suggests that the operation is done at the wrong level. Do the pin configuration as you parse DT data, the same way you set up i2c devices registers in of_i2c.c, and leave the individual drivers that do not care about specifics alone. Exactly this can be done with pinctrl hogs. The problem with that is that it removes the cross-reference between the device and it's pinctrl handle (also from the device tree). Instead the pinctrl handle gets referenced to the pin controller itself. So from a modelling perpective this looks a bit ugly. So we have two kinds of ugly: - Sprinke devm_pinctrl_get_select_default() over all drivers which makes pinctrl handles properly reference their devices - Use hogs and loose coupling between pinctrl handles and their devices A third alternative as outlined is to use notifiers and some resource core in drivers/base/* OK, so with drivers/base/, have you considered doing default pinctrl selection in bus's probe() methods? Yo would select the default configuration before starting probing the device and maybe select idle when probe fails or device is unbound? That would still keep the link between device object and pinctrl and there less busses than device drivers out there. it starts to become confusing after a while. I mean, there's a reason why all drivers explictly call pm_runtim_enable(), right ? From a first thought, one could think of just yanking that into bus' probe() as you may suggest, but sometimes the device is already enabled, so we need extra tricks: pm_runtime_set_active(); pm_runtime_enable(); pm_runtime_get(); the same could happen with pinctrl eventually. What if a device needs to do something else (an errata fix as an example) before requesting pinctrl's default state ? -- balbi signature.asc Description: Digital signature
Re: [PATCH 1/4] dmaengine: dw_dmac: use helper macro module_platform_driver()
On Wed, Oct 10, 2012 at 12:21:04PM +0300, Andy Shevchenko wrote: On Wed, Oct 10, 2012 at 12:08 PM, viresh kumar viresh.ku...@linaro.org wrote: On Wed, Oct 10, 2012 at 2:34 PM, Andy Shevchenko andriy.shevche...@linux.intel.com wrote: On Tue, 2012-10-02 at 14:41 +0300, Andy Shevchenko wrote: From: Heikki Krogerus heikki.kroge...@linux.intel.com Since v3.2 we have nice macro to define the platform driver's init and exit calls. This patch simplifies the dw_dmac driver by using that macro. Actually we can't do this. It will break initialization of some other drivers. why? We have spi, i2c and hsuart devices connected to the DMA controller. In case we would like to use DMA we have to have the dw_dmac loaded before them. Currently we have spi driver on subsys_initcall level, and Mika, who is developing it, will change to module_init_call level. However, it will just hide the potential issue. He also tried to use deferred module loading, but we don't know if it's good solution or not, and that solution requires something to stop deferring at some moment. Might be we missed something and there is a better solution. if they can only work with DMA, they should return -EPROBE_DEFER so their probe() function can be called after DMA driver has finished probing. -- balbi signature.asc Description: Digital signature
Re: [PATCH 1/4] dmaengine: dw_dmac: use helper macro module_platform_driver()
Hi, On Wed, Oct 10, 2012 at 03:52:40PM +0300, Andy Shevchenko wrote: On Wed, Oct 10, 2012 at 3:40 PM, Felipe Balbi ba...@ti.com wrote: On Wed, Oct 10, 2012 at 12:21:04PM +0300, Andy Shevchenko wrote: On Wed, Oct 10, 2012 at 12:08 PM, viresh kumar viresh.ku...@linaro.org wrote: On Wed, Oct 10, 2012 at 2:34 PM, Andy Shevchenko andriy.shevche...@linux.intel.com wrote: On Tue, 2012-10-02 at 14:41 +0300, Andy Shevchenko wrote: From: Heikki Krogerus heikki.kroge...@linux.intel.com Since v3.2 we have nice macro to define the platform driver's init and exit calls. This patch simplifies the dw_dmac driver by using that macro. Actually we can't do this. It will break initialization of some other drivers. why? We have spi, i2c and hsuart devices connected to the DMA controller. In case we would like to use DMA we have to have the dw_dmac loaded before them. Currently we have spi driver on subsys_initcall level, and Mika, who is developing it, will change to module_init_call level. However, it will just hide the potential issue. He also tried to use deferred module loading, but we don't know if it's good solution or not, and that solution requires something to stop deferring at some moment. Might be we missed something and there is a better solution. if they can only work with DMA, they should return -EPROBE_DEFER so their probe() function can be called after DMA driver has finished probing. They could work either with DMA or via PIO mode. How does the driver know when to stop to return -EPROBE_DEFER? Why would you even allow to work as PIO-only ? Who would even want to use the driver as PIO only ? In any case, you can add a Kconfig choice like WHATEVER_PIO_ONLY and only return -EPROBE_DEFER ifndef WHATEVER_PIO_ONLY. -- balbi signature.asc Description: Digital signature
Re: [PATCH 1/4] dmaengine: dw_dmac: use helper macro module_platform_driver()
Hi, On Wed, Oct 10, 2012 at 04:42:00PM +0300, Felipe Balbi wrote: On Wed, Oct 10, 2012 at 03:52:40PM +0300, Andy Shevchenko wrote: On Wed, Oct 10, 2012 at 3:40 PM, Felipe Balbi ba...@ti.com wrote: On Wed, Oct 10, 2012 at 12:21:04PM +0300, Andy Shevchenko wrote: On Wed, Oct 10, 2012 at 12:08 PM, viresh kumar viresh.ku...@linaro.org wrote: On Wed, Oct 10, 2012 at 2:34 PM, Andy Shevchenko andriy.shevche...@linux.intel.com wrote: On Tue, 2012-10-02 at 14:41 +0300, Andy Shevchenko wrote: From: Heikki Krogerus heikki.kroge...@linux.intel.com Since v3.2 we have nice macro to define the platform driver's init and exit calls. This patch simplifies the dw_dmac driver by using that macro. Actually we can't do this. It will break initialization of some other drivers. why? We have spi, i2c and hsuart devices connected to the DMA controller. In case we would like to use DMA we have to have the dw_dmac loaded before them. Currently we have spi driver on subsys_initcall level, and Mika, who is developing it, will change to module_init_call level. However, it will just hide the potential issue. He also tried to use deferred module loading, but we don't know if it's good solution or not, and that solution requires something to stop deferring at some moment. Might be we missed something and there is a better solution. if they can only work with DMA, they should return -EPROBE_DEFER so their probe() function can be called after DMA driver has finished probing. They could work either with DMA or via PIO mode. How does the driver know when to stop to return -EPROBE_DEFER? Why would you even allow to work as PIO-only ? Who would even want to use the driver as PIO only ? In any case, you can add a Kconfig choice like WHATEVER_PIO_ONLY and only return -EPROBE_DEFER ifndef WHATEVER_PIO_ONLY. yet another comment is that any user of this dma engine will have to pass the filter function as argument to request_channel, which means Modules.dep will make sure for dw_dmac to probe before its users. What am I missing here ? -- balbi signature.asc Description: Digital signature
Re: [PATCH] Revert serial: omap: fix software flow control
On Wed, Oct 24, 2012 at 11:57:42AM -0700, Greg KH wrote: On Wed, Oct 24, 2012 at 01:02:55PM +0300, Felipe Balbi wrote: Hi Greg, On Tue, Oct 16, 2012 at 08:13:59AM -0700, Tony Lindgren wrote: * Felipe Balbi ba...@ti.com [121016 07:16]: This reverts commit 957ee7270d632245b43f6feb0e70d9a5e9ea6cf6 (serial: omap: fix software flow control). As Russell has pointed out, that commit isn't fixing Software Flow Control at all, and it actually makes it even more broken. It was agreed to revert this commit and use Russell's latest UART patches instead. Cc: Russell King li...@arm.linux.org.uk Signed-off-by: Felipe Balbi ba...@ti.com This seems like the best way to go for the -rc series: Acked-by: Tony Lindgren t...@atomide.com Any chance you can pick this one up for v3.7-rc3 ? Now queued up, sorry for the delay. no problems, thanks a lot. -- balbi signature.asc Description: Digital signature
Re: [PATCHv2] Input: omap4-keypad: Add pinctrl support
Hi, On Wed, Oct 24, 2012 at 10:28:46AM -0700, Dmitry Torokhov wrote: for more complex pinctrl use cases. These are my dogfood drivers ... Most of these will request more than one state and switch the driver between these different states at runtime, in these examples for power saving there are states named default, sleep and in the I2C driver also idle. These examples are more typical to how the ux500 platform will look, also the SKE input driver will move the devise to sleep/default states but we need to merge PM code before we can do that. I do not say that no drivers should ever touch pinctrl, just that most of them do not have to if you have other layers to the right thing for them. It will be a much bigger mess. Some drivers don't need to care about pinctrl because drivers/base handle it for them, while some others will need a way to tell drivers/base hey, don't touch pinctrl at all because I know what I'm doing and that has to happen before probe() too, otherwise it's already too late and, according to what you suggest, drivers/base will already have touched pinctrl. The only way I see would be to add an extra dont_touch_my_pins field to every driver structure in the kernel. Clearly what you say is nonsense. -- balbi signature.asc Description: Digital signature
Re: [PATCHv2] Input: omap4-keypad: Add pinctrl support
Hi, On Wed, Oct 24, 2012 at 10:58:53AM -0700, Dmitry Torokhov wrote: On Wednesday, October 24, 2012 07:57:49 PM Felipe Balbi wrote: Hi, On Wed, Oct 24, 2012 at 09:18:01AM -0700, Dmitry Torokhov wrote: On Wed, Oct 24, 2012 at 02:54:23PM +0200, Linus Walleij wrote: On Tue, Oct 23, 2012 at 10:02 PM, Dmitry Torokhov dmitry.torok...@gmail.com wrote: I have seen just in a few days 3 or 4 drivers having exactly the same change - call to devm_pinctrl_get_select_default(), and I guess I will receive the same patches for the rest of input drivers shortly. This suggests that the operation is done at the wrong level. Do the pin configuration as you parse DT data, the same way you set up i2c devices registers in of_i2c.c, and leave the individual drivers that do not care about specifics alone. Exactly this can be done with pinctrl hogs. The problem with that is that it removes the cross-reference between the device and it's pinctrl handle (also from the device tree). Instead the pinctrl handle gets referenced to the pin controller itself. So from a modelling perpective this looks a bit ugly. So we have two kinds of ugly: - Sprinke devm_pinctrl_get_select_default() over all drivers which makes pinctrl handles properly reference their devices - Use hogs and loose coupling between pinctrl handles and their devices A third alternative as outlined is to use notifiers and some resource core in drivers/base/* OK, so with drivers/base/, have you considered doing default pinctrl selection in bus's probe() methods? Yo would select the default configuration before starting probing the device and maybe select idle when probe fails or device is unbound? That would still keep the link between device object and pinctrl and there less busses than device drivers out there. it starts to become confusing after a while. I mean, there's a reason why all drivers explictly call pm_runtim_enable(), right ? Right. Because not all of them support runtime PM and quite usually their PM methods are not ready to go until initialization is complete. And again, the driver here controls its own behavior. likewise not all devices will need pin muxing, those which do (granted, an increasing number of them since transistor size continue to shrink, allowing chip manufacturers to pack more features inside a single die, while the number of external pins/balls remain the same), will call pinctrl to setup muxing right. From a first thought, one could think of just yanking that into bus' probe() as you may suggest, but sometimes the device is already enabled, so we need extra tricks: pm_runtime_set_active(); pm_runtime_enable(); pm_runtime_get(); the same could happen with pinctrl eventually. What if a device needs to do something else (an errata fix as an example) before requesting pinctrl's default state ? That is a valid concern and we'll need to find a compromise here. As I said, WHAT ?? Silicon erratas are not a valid concern ? Power waste isn't a valid concern ? Tell that to the millions of devices shipped with Linux everyday. Power usage if it's the top concern in any product, is right there as the top five. Likewise for silicon erratas. Let's face it, just like SW, HW has bugs; the difference is that no company will continue to do several spins of an ASIC just because some SW engineer doesn't get concerned about a silicon bug. It's just too expensive to re-spin the ASIC. And even if we get another revision of the ASIC, we still need to support the older version as there might be cellphones, laser welding machines, IPTVs and whatever product already shipped. I am not saying that no driver should ever touch pinctrl. I can see, for example, input drivers actually disabling pins until they are -open()ed, in addition of doing that at [runtime] suspend/resume. But it would be nice if we could have dumb drivers not care about pins. Like I replied on another sub-thread, this will just create exceptions to the rule which is far more messy than having a couple of extra lines of code in a few drivers. We can even argue that eventually all drivers will need to toggle pins in runtime in order to save that extra microwatt of runtime power consumption, so why bother adding exceptions ? In fact, we already have the exception: drivers which don't need to fiddle with pin muxing, just don't use pinctrl. The ones you're receiving today are the one which, for whatever reason, need to make sure pin muxing is right. If it's not toggling in runtime, it might just be because $AUTHOR decided that it would be best to do thing in small steps (don't we all agree with that ?). Maybe he thought that changing pins in runtime could cause problems, so let's get bare minimum in mainline and work towards optimizations in parallel. All in all, I don't see why
Re: [PATCHv2] Input: omap4-keypad: Add pinctrl support
Hi, On Wed, Oct 24, 2012 at 12:38:44PM -0700, Dmitry Torokhov wrote: On Wednesday, October 24, 2012 10:10:42 PM Felipe Balbi wrote: That is a valid concern and we'll need to find a compromise here. As I said, WHAT ?? Silicon erratas are not a valid concern ? Power waste isn't a valid concern ? Tell that to the millions of devices shipped with Linux everyday. Power usage if it's the top concern in any product, is right there as the top five. Likewise for silicon erratas. I think we should come back to this discussion when you get more coffee and start parsing other party e-mails properly. indeed. I really misparsed it. My bad. comments withdrawn. -- balbi signature.asc Description: Digital signature
Re: [PATCH v2 1/3] drivers: bus: ocp2scp: add pdata support
Hi, On Wed, Oct 24, 2012 at 05:48:07PM -0700, Tony Lindgren wrote: * Tony Lindgren t...@atomide.com [121016 09:53]: * Kishon Vijay Abraham I kis...@ti.com [121007 23:01]: ocp2scp was not having pdata support which makes *musb* fail for non-dt boot in OMAP platform. The pdata will have information about the devices that is connected to ocp2scp. ocp2scp driver will now make use of this information to create the devices that is attached to ocp2scp. Signed-off-by: Kishon Vijay Abraham I kis...@ti.com This fixes the regression on my panda es for musb port: Acked-by: Tony Lindgren t...@atomide.com Looks like nobody has picked this one up and we need it to fix the musb regression on omap, so I'll queue these up. I don't seem to have the patches around in any mailbox :-( -- balbi signature.asc Description: Digital signature
Re: [PATCH v6] Enable USB peripheral mode on dm365 EVM
Hi, On Wed, Oct 10, 2012 at 02:33:32PM +0200, Constantine Shulyupin wrote: From: Constantine Shulyupin co...@makelinux.com Sets USB PHY clock source to 24 MHz clock and call USB configuration from board initialization. Tested with OTG configuration, usb gadget g_zero on DM365 EVM connected to PC. References: Definition of USB_PHY_CTRL and PHYCLKFREQ: - http://www.makelinux.com/lib/ti/DM36x_ARM/doc-141 Original patch by miguel.agui...@ridgerun.com three years ago: - http://www.mail-archive.com/davinci-linux-open-source@linux.davincidsp.com/msg14741.html Signed-off-by: Constantine Shulyupin co...@makelinux.com --- Note: Changelog Changes since v5 http://www.spinics.net/lists/kernel/msg1413120.html accordingy feedback of nsek...@ti.com http://www.spinics.net/lists/kernel/msg1414914.html - phy configuration moved to drivers/usb/musb/davinci.c - USB_OTG configuration is submitted in separated patch: http://www.spinics.net/lists/kernel/msg1414964.html - Setting current limit to 1000 mA. Any way the current is limited to 510 mA in davinci_setup_usb. Changes since v4 http://www.spinics.net/lists/kernel/msg1412995.html - removed fix of dev_info in musb_init_controller Changes since v3 http://www.spinics.net/lists/kernel/msg1412544.html: - removed optional altering of pr_info Changes since v1 http://marc.info/?l=linux-kernelm=130894150803661w=2: - removed optional code and reordered - removed alternation of GPIO33, which is multiplexed with DRVVBUS, because is not need for peripheral USB This patch is based on code from projects Arago, Angstom and RidgeRun. --- arch/arm/mach-davinci/board-dm365-evm.c |2 ++ drivers/usb/musb/davinci.c |3 +++ drivers/usb/musb/davinci.h |1 + 3 files changed, 6 insertions(+) diff --git a/arch/arm/mach-davinci/board-dm365-evm.c b/arch/arm/mach-davinci/board-dm365-evm.c index 688a9c5..ba5ffc1 100644 --- a/arch/arm/mach-davinci/board-dm365-evm.c +++ b/arch/arm/mach-davinci/board-dm365-evm.c @@ -38,6 +38,7 @@ #include mach/mmc.h #include mach/nand.h #include mach/keyscan.h +#include mach/usb.h #include media/tvp514x.h @@ -610,6 +611,7 @@ static __init void dm365_evm_init(void) dm365_init_spi0(BIT(0), dm365_evm_spi_info, ARRAY_SIZE(dm365_evm_spi_info)); + davinci_setup_usb(1000, 8); } MACHINE_START(DAVINCI_DM365_EVM, DaVinci DM365 EVM) diff --git a/drivers/usb/musb/davinci.c b/drivers/usb/musb/davinci.c index 472c8b4..af09ebf 100644 --- a/drivers/usb/musb/davinci.c +++ b/drivers/usb/musb/davinci.c @@ -428,6 +428,9 @@ static int davinci_musb_init(struct musb *musb) __raw_writel(deepsleep, DM355_DEEPSLEEP); } + if (machine_is_davinci_dm365_evm()) + writel(readl(USB_PHY_CTRL) | USBPHY_CLKFREQ_24MHZ, USB_PHY_CTRL); no such checks in drivers. Please find another way to do this. -- balbi signature.asc Description: Digital signature
