Re: [PATCH 0/2] Introduce usb charger framework to deal with the usb gadget power negotation

2015-08-08 Thread Baolin Wang
On 8 August 2015 at 01:52, Greg KH gre...@linuxfoundation.org wrote:
 On Fri, Aug 07, 2015 at 04:19:40PM +0800, Baolin Wang wrote:
 On 7 August 2015 at 13:34, Peter Chen peter.c...@freescale.com wrote:
  On Thu, Aug 06, 2015 at 03:03:47PM +0800, Baolin Wang wrote:
  Currently the Linux kernel does not provide any standard integration of 
  this
  feature that integrates the USB subsystem with the system power regulation
  provided by PMICs meaning that either vendors must add this in their 
  kernels
  or USB gadget devices based on Linux (such as mobile phones) may not 
  behave
  as they should.
 
  Providing a standard framework for doing this in the kernel.
 
  Baolin, thanks for introducing a framework for doing it, we do support
  USB Charger for chipidea driver at internal tree, but it is specific
  for imx, and still have some problems to upstream due to need to
  change some common code.
 
  One suggestion, would you add your user next time? In that case, we can
  know better for this framework.
 

 Peter, Thanks for your reviewing and comments. Now I just introduce
 the framework to review for more feedbacks and do not have a useful
 user to use it. I just can show you some example code to show how to
 use it. Thanks.

 Without a real, in-tree user, I can not accept this code.  We don't add
 frameworks for non-existant things, otherwise it will be instantly
 ripped out the next kernel release.

 Please come up with at least 2 users, ideally 3, otherwise there's no
 real way to know if the framework is sufficient.


OK, I'll try to come up with 2 or 3 users. Thanks.

 thanks,

 greg k-h



-- 
Baolin.wang
Best Regards
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 2/2] gadget: Support for the usb charger framework

2015-08-08 Thread Baolin Wang
On 8 August 2015 at 01:53, Greg KH gre...@linuxfoundation.org wrote:
 On Fri, Aug 07, 2015 at 05:22:47PM +0800, Baolin Wang wrote:
 On 7 August 2015 at 17:07, Peter Chen peter.c...@freescale.com wrote:
 
/**
 * struct usb_udc - describes one usb device controller @@ -127,12
   +128,45 @@ void usb_gadget_giveback_request(struct usb_ep *ep,  }
   EXPORT_SYMBOL_GPL(usb_gadget_giveback_request);
  
   +int usb_gadget_register_notify(struct usb_gadget *gadget,
   +struct notifier_block *nb) {
   + unsigned long flags;
   + int ret;
   +
   + spin_lock_irqsave(gadget-lock, flags);
  
   I find you use so many spin_lock_irqsave, any reasons for that?
   Why mutex_lock can't be used?
  
 
  The spin_lock_irqsave() can make it as a atomic notifier, that can make 
  sure the
  gadget state event can be quickly reported to the user who register a 
  notifier
  on the gadget device. Is it OK?
 
 
  I don't think it is a good reason, spin_lock_irqsave is usually used for 
  protecting
  data which is accessed at atomic environment.
 

 Yes, we want the notify process is a atomic environment which do not
 want to be interrupted by irq or other things to make the notice can
 be quickly reported to the user.

 No, this is a slow event, you don't need to notify anyone under atomic
 context, that's crazy.

 Also i think the notify process is less cost, thus i use the spinlock. 
 Thanks.

 No, use a mutex please, that's the safe thing.  This is not
 time-critical code at all.


OK, Thanks for your comments and will fix the lock thing.

 thanks,

 greg k-h



-- 
Baolin.wang
Best Regards
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH] usb: core: hub.c: Removed some warnings generated by checkpatch.pl

2015-08-08 Thread Chase Metzger
Removed some checkpatch.pl warnings saying there was an unwanted space between
function names and their arguments.

Signed-off-by: Chase Metzger chasemetzge...@gmail.com

---
 drivers/usb/core/hub.c | 40 
 1 file changed, 20 insertions(+), 20 deletions(-)

diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index 73dfa19..9387cc20 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -50,8 +50,8 @@ DEFINE_MUTEX(usb_port_peer_mutex);
 
 /* cycle leds on hubs that aren't blinking for attention */
 static bool blinkenlights = 0;
-module_param (blinkenlights, bool, S_IRUGO);
-MODULE_PARM_DESC (blinkenlights, true to cycle leds on hubs);
+module_param(blinkenlights, bool, S_IRUGO);
+MODULE_PARM_DESC(blinkenlights, true to cycle leds on hubs);
 
 /*
  * Device SATA8000 FW1.0 from DATAST0R Technology Corp requires about
@@ -439,7 +439,7 @@ static void set_port_led(struct usb_hub *hub, int port1, 
int selector)
 
 #defineLED_CYCLE_PERIOD((2*HZ)/3)
 
-static void led_work (struct work_struct *work)
+static void led_work(struct work_struct *work)
 {
struct usb_hub  *hub =
container_of(work, struct usb_hub, leds.work);
@@ -646,7 +646,7 @@ static void hub_irq(struct urb *urb)
 
default:/* presumably an error */
/* Cause a hub reset after 10 consecutive errors */
-   dev_dbg (hub-intfdev, transfer -- %d\n, status);
+   dev_dbg(hub-intfdev, transfer -- %d\n, status);
if ((++hub-nerrors  10) || hub-error)
goto resubmit;
hub-error = status;
@@ -671,14 +671,14 @@ resubmit:
if (hub-quiescing)
return;
 
-   if ((status = usb_submit_urb (hub-urb, GFP_ATOMIC)) != 0
+   if ((status = usb_submit_urb(hub-urb, GFP_ATOMIC)) != 0
 status != -ENODEV  status != -EPERM)
-   dev_err (hub-intfdev, resubmit -- %d\n, status);
+   dev_err(hub-intfdev, resubmit -- %d\n, status);
 }
 
 /* USB 2.0 spec Section 11.24.2.3 */
 static inline int
-hub_clear_tt_buffer (struct usb_device *hdev, u16 devinfo, u16 tt)
+hub_clear_tt_buffer(struct usb_device *hdev, u16 devinfo, u16 tt)
 {
/* Need to clear both directions for control ep */
if (((devinfo  11)  USB_ENDPOINT_XFERTYPE_MASK) ==
@@ -706,7 +706,7 @@ static void hub_tt_work(struct work_struct *work)
container_of(work, struct usb_hub, tt.clear_work);
unsigned long   flags;
 
-   spin_lock_irqsave (hub-tt.lock, flags);
+   spin_lock_irqsave(hub-tt.lock, flags);
while (!list_empty(hub-tt.clear_list)) {
struct list_head*next;
struct usb_tt_clear *clear;
@@ -715,14 +715,14 @@ static void hub_tt_work(struct work_struct *work)
int status;
 
next = hub-tt.clear_list.next;
-   clear = list_entry (next, struct usb_tt_clear, clear_list);
-   list_del (clear-clear_list);
+   clear = list_entry(next, struct usb_tt_clear, clear_list);
+   list_del(clear-clear_list);
 
/* drop lock so HCD can concurrently report other TT errors */
-   spin_unlock_irqrestore (hub-tt.lock, flags);
-   status = hub_clear_tt_buffer (hdev, clear-devinfo, clear-tt);
+   spin_unlock_irqrestore(hub-tt.lock, flags);
+   status = hub_clear_tt_buffer(hdev, clear-devinfo, clear-tt);
if (status  status != -ENODEV)
-   dev_err (hdev-dev,
+   dev_err(hdev-dev,
clear tt %d (%04x) error %d\n,
clear-tt, clear-devinfo, status);
 
@@ -734,7 +734,7 @@ static void hub_tt_work(struct work_struct *work)
kfree(clear);
spin_lock_irqsave(hub-tt.lock, flags);
}
-   spin_unlock_irqrestore (hub-tt.lock, flags);
+   spin_unlock_irqrestore(hub-tt.lock, flags);
 }
 
 /**
@@ -797,7 +797,7 @@ int usb_hub_clear_tt_buffer(struct urb *urb)
 */
clear = kmalloc(sizeof *clear, GFP_ATOMIC);
if (clear == NULL) {
-   dev_err (udev-dev, can't save CLEAR_TT_BUFFER state\n);
+   dev_err(udev-dev, can't save CLEAR_TT_BUFFER state\n);
/* FIXME recover somehow ... RESET_TT? */
return -ENOMEM;
}
@@ -806,10 +806,10 @@ int usb_hub_clear_tt_buffer(struct urb *urb)
clear-tt = tt-multi ? udev-ttport : 1;
clear-devinfo = usb_pipeendpoint (pipe);
clear-devinfo |= udev-devnum  4;
-   clear-devinfo |= usb_pipecontrol (pipe)
+   clear-devinfo |= usb_pipecontrol(pipe)
? (USB_ENDPOINT_XFER_CONTROL  11)
: (USB_ENDPOINT_XFER_BULK  11);
-   if (usb_pipein (pipe))
+   if 

Re: [PATCH v5 1/2] usb: make xhci platform driver use 64 bit or 32 bit DMA

2015-08-08 Thread Russell King - ARM Linux
On Fri, Aug 07, 2015 at 08:18:48PM -0700, Duc Dang wrote:
 diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c
 index 890ad9d..5d03f8b 100644
 --- a/drivers/usb/host/xhci-plat.c
 +++ b/drivers/usb/host/xhci-plat.c
 @@ -93,14 +93,14 @@ static int xhci_plat_probe(struct platform_device *pdev)
   if (irq  0)
   return -ENODEV;
  
 - /* Initialize dma_mask and coherent_dma_mask to 32-bits */
 - ret = dma_set_coherent_mask(pdev-dev, DMA_BIT_MASK(32));
 - if (ret)
 - return ret;
 - if (!pdev-dev.dma_mask)
 - pdev-dev.dma_mask = pdev-dev.coherent_dma_mask;
 - else
 - dma_set_mask(pdev-dev, DMA_BIT_MASK(32));
 + /* Try setting the coherent_dma_mask to 64 bits, then try 32 bits */
 + ret = dma_set_mask_and_coherent(pdev-dev, DMA_BIT_MASK(64));
 + if (ret) {
 + ret = dma_set_mask_and_coherent(pdev-dev, DMA_BIT_MASK(32));
 + if (ret)
 + return ret;
 + }

Note that dma_set_mask_and_coherent() and the original code are not
equivalent because of this:

if (!pdev-dev.dma_mask)
pdev-dev.dma_mask = pdev-dev.coherent_dma_mask;

If we know that pdev-dev.dma_mask will always be initialised at this
point, then the above change is fine.  If not, it's introducing a
regression - dma_set_mask_and_coherent() will fail if pdev-dev.dma_mask
is NULL (depending on the architectures implementation of dma_set_mask()).

Prefixing the above change with the two lines I mention above would
ensure equivalent behaviour.  Even if we do want to get rid of this,
I'd advise to do it as a separate patch after this change, which can
be independently reverted if there's problems with its removal.

-- 
FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up
according to speedtest.net.
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v9 1/7] usb: interface authorization: Declare authorized attribute

2015-08-08 Thread Stefan Koch
The attribute authorized shows the authorization state for an interface.

Signed-off-by: Stefan Koch sk...@suse.de
---
 include/linux/usb.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/include/linux/usb.h b/include/linux/usb.h
index 447fe29..3deccab 100644
--- a/include/linux/usb.h
+++ b/include/linux/usb.h
@@ -178,6 +178,7 @@ struct usb_interface {
unsigned needs_altsetting0:1;   /* switch to altsetting 0 is pending */
unsigned needs_binding:1;   /* needs delayed unbind/rebind */
unsigned resetting_device:1;/* true: bandwidth alloc after reset */
+   unsigned authorized:1;  /* used for interface authorization */
 
struct device dev;  /* interface specific device info */
struct device *usb_dev;
-- 
2.1.4

--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v9 0/7] usb: interface authorization

2015-08-08 Thread Stefan Koch
This patch introduces an interface authorization for USB devices.
The kernel supports a device authorization because of wireless USB.

But the new interface authorization allows to authorize or deauthorize
individual interfaces instead authorization or deauthorize a whole device.

Therefore the authorized attribute is introduced for each interface.

Each patch depends on all patches with a lesser number.

v5 was acked-by Alan Stern:
http://comments.gmane.org/gmane.linux.usb.general/127144
http://permalink.gmane.org/gmane.linux.usb.general/127151

Changes since v8:
- Implemented suggestions from Greg K-H (number and doc issue)

Changes since v7:
- Implemented suggestions from Alan Stern and Sergei Shtylyov

Changes since v6:
- Implemented suggestions from Alan Stern and Sergei Shtylyov

Changes since v5:
- Implemented suggestions from Greg K-H
- Changed device authorization to save the default bit in the same flag as the 
interface authorization does this
  (recommended by Alan Stern 
http://permalink.gmane.org/gmane.linux.usb.general/127086)


Stefan Koch (7):
  usb: interface authorization: Declare authorized attribute
  usb: interface authorization: Introduces the default interface
authorization
  usb: interface authorization: Control interface probing and claiming
  usb: interface authorization: Introduces the USB interface
authorization
  usb: interface authorization: SysFS part of USB interface
authorization
  usb: interface authorization: Documentation part
  usb: interface authorization: Use a flag for the default device
authorization

 Documentation/ABI/testing/sysfs-bus-usb | 22 ++
 Documentation/usb/authorization.txt | 34 ++
 drivers/usb/core/driver.c   |  8 
 drivers/usb/core/hcd.c  | 78 -
 drivers/usb/core/message.c  | 39 +
 drivers/usb/core/sysfs.c| 36 +++
 drivers/usb/core/usb.c  |  2 +-
 drivers/usb/core/usb.h  |  2 +
 include/linux/usb.h |  1 +
 include/linux/usb/hcd.h | 25 ---
 10 files changed, 229 insertions(+), 18 deletions(-)

-- 
2.1.4

--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v9 2/7] usb: interface authorization: Introduces the default interface authorization

2015-08-08 Thread Stefan Koch
Interfaces are allowed per default.
This can disabled or enabled (again) by writing 0 or 1 to
/sys/bus/usb/devices/usbX/interface_authorized_default

Signed-off-by: Stefan Koch sk...@suse.de
---
 drivers/usb/core/hcd.c | 47 ++
 drivers/usb/core/message.c |  1 +
 include/linux/usb/hcd.h|  9 +
 3 files changed, 57 insertions(+)

diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c
index cbcd092..84b5923 100644
--- a/drivers/usb/core/hcd.c
+++ b/drivers/usb/core/hcd.c
@@ -882,9 +882,53 @@ static ssize_t authorized_default_store(struct device *dev,
 }
 static DEVICE_ATTR_RW(authorized_default);
 
+/*
+ * interface_authorized_default_show - show default authorization status
+ * for USB interfaces
+ *
+ * note: interface_authorized_default is the default value
+ *   for initializing the authorized attribute of interfaces
+ */
+static ssize_t interface_authorized_default_show(struct device *dev,
+   struct device_attribute *attr, char *buf)
+{
+   struct usb_device *usb_dev = to_usb_device(dev);
+   struct usb_hcd *hcd = bus_to_hcd(usb_dev-bus);
+
+   return sprintf(buf, %u\n, !!HCD_INTF_AUTHORIZED(hcd));
+}
+
+/*
+ * interface_authorized_default_store - store default authorization status
+ * for USB interfaces
+ *
+ * note: interface_authorized_default is the default value
+ *   for initializing the authorized attribute of interfaces
+ */
+static ssize_t interface_authorized_default_store(struct device *dev,
+   struct device_attribute *attr, const char *buf, size_t count)
+{
+   struct usb_device *usb_dev = to_usb_device(dev);
+   struct usb_hcd *hcd = bus_to_hcd(usb_dev-bus);
+   int rc = count;
+   bool val;
+
+   if (strtobool(buf, val) != 0)
+   return -EINVAL;
+
+   if (val)
+   set_bit(HCD_FLAG_INTF_AUTHORIZED, hcd-flags);
+   else
+   clear_bit(HCD_FLAG_INTF_AUTHORIZED, hcd-flags);
+
+   return rc;
+}
+static DEVICE_ATTR_RW(interface_authorized_default);
+
 /* Group all the USB bus attributes */
 static struct attribute *usb_bus_attrs[] = {
dev_attr_authorized_default.attr,
+   dev_attr_interface_authorized_default.attr,
NULL,
 };
 
@@ -2682,6 +2726,9 @@ int usb_add_hcd(struct usb_hcd *hcd,
hcd-authorized_default = authorized_default;
set_bit(HCD_FLAG_HW_ACCESSIBLE, hcd-flags);
 
+   /* per default all interfaces are authorized */
+   set_bit(HCD_FLAG_INTF_AUTHORIZED, hcd-flags);
+
/* HC is in reset state, but accessible.  Now do the one-time init,
 * bottom up so that hcds can customize the root hubs before hub_wq
 * starts talking to them.  (Note, bus id is assigned early too.)
diff --git a/drivers/usb/core/message.c b/drivers/usb/core/message.c
index f368d20..3d25d89 100644
--- a/drivers/usb/core/message.c
+++ b/drivers/usb/core/message.c
@@ -1807,6 +1807,7 @@ free_interfaces:
intfc = cp-intf_cache[i];
intf-altsetting = intfc-altsetting;
intf-num_altsetting = intfc-num_altsetting;
+   intf-authorized = !!HCD_INTF_AUTHORIZED(hcd);
kref_get(intfc-ref);
 
alt = usb_altnum_to_altsetting(intf, 0);
diff --git a/include/linux/usb/hcd.h b/include/linux/usb/hcd.h
index c9aa779..e56c6b2 100644
--- a/include/linux/usb/hcd.h
+++ b/include/linux/usb/hcd.h
@@ -120,6 +120,7 @@ struct usb_hcd {
 #define HCD_FLAG_WAKEUP_PENDING4   /* root hub is 
resuming? */
 #define HCD_FLAG_RH_RUNNING5   /* root hub is running? */
 #define HCD_FLAG_DEAD  6   /* controller has died? */
+#define HCD_FLAG_INTF_AUTHORIZED   7   /* authorize interfaces? */
 
/* The flags can be tested using these macros; they are likely to
 * be slightly faster than test_bit().
@@ -131,6 +132,14 @@ struct usb_hcd {
 #define HCD_RH_RUNNING(hcd)((hcd)-flags  (1U  HCD_FLAG_RH_RUNNING))
 #define HCD_DEAD(hcd)  ((hcd)-flags  (1U  HCD_FLAG_DEAD))
 
+   /*
+* Specifies if interfaces are authorized by default
+* or they require explicit user space authorization; this bit is
+* settable through /sys/class/usb_host/X/interface_authorized_default
+*/
+#define HCD_INTF_AUTHORIZED(hcd) \
+   ((hcd)-flags  (1U  HCD_FLAG_INTF_AUTHORIZED))
+
/* Flags that get set only during HCD registration or removal. */
unsignedrh_registered:1;/* is root hub registered? */
unsignedrh_pollable:1;  /* may we poll the root hub? */
-- 
2.1.4

--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v9 3/7] usb: interface authorization: Control interface probing and claiming

2015-08-08 Thread Stefan Koch
Driver probings and interface claims get rejected
if an interface is not authorized.

Signed-off-by: Stefan Koch sk...@suse.de
---
 drivers/usb/core/driver.c | 8 
 1 file changed, 8 insertions(+)

diff --git a/drivers/usb/core/driver.c b/drivers/usb/core/driver.c
index 818369a..d542d43 100644
--- a/drivers/usb/core/driver.c
+++ b/drivers/usb/core/driver.c
@@ -295,6 +295,10 @@ static int usb_probe_interface(struct device *dev)
if (udev-authorized == 0) {
dev_err(intf-dev, Device is not authorized for usage\n);
return error;
+   } else if (intf-authorized == 0) {
+   dev_err(intf-dev, Interface %d is not authorized for 
usage\n,
+   intf-altsetting-desc.bInterfaceNumber);
+   return error;
}
 
id = usb_match_dynamic_id(intf, driver);
@@ -507,6 +511,10 @@ int usb_driver_claim_interface(struct usb_driver *driver,
if (dev-driver)
return -EBUSY;
 
+   /* reject claim if not iterface is not authorized */
+   if (!iface-authorized)
+   return -ENODEV;
+
udev = interface_to_usbdev(iface);
 
dev-driver = driver-drvwrap.driver;
-- 
2.1.4

--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v9 7/7] usb: interface authorization: Use a flag for the default device authorization

2015-08-08 Thread Stefan Koch
With this patch a flag instead of a variable
is used for the default device authorization.

Signed-off-by: Stefan Koch sk...@suse.de
---
 drivers/usb/core/hcd.c  | 31 +--
 drivers/usb/core/usb.c  |  2 +-
 include/linux/usb/hcd.h | 16 +---
 3 files changed, 31 insertions(+), 18 deletions(-)

diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c
index 84b5923..a567647 100644
--- a/drivers/usb/core/hcd.c
+++ b/drivers/usb/core/hcd.c
@@ -854,10 +854,10 @@ static ssize_t authorized_default_show(struct device *dev,
 {
struct usb_device *rh_usb_dev = to_usb_device(dev);
struct usb_bus *usb_bus = rh_usb_dev-bus;
-   struct usb_hcd *usb_hcd;
+   struct usb_hcd *hcd;
 
-   usb_hcd = bus_to_hcd(usb_bus);
-   return snprintf(buf, PAGE_SIZE, %u\n, usb_hcd-authorized_default);
+   hcd = bus_to_hcd(usb_bus);
+   return snprintf(buf, PAGE_SIZE, %u\n, !!HCD_DEV_AUTHORIZED(hcd));
 }
 
 static ssize_t authorized_default_store(struct device *dev,
@@ -868,12 +868,16 @@ static ssize_t authorized_default_store(struct device 
*dev,
unsigned val;
struct usb_device *rh_usb_dev = to_usb_device(dev);
struct usb_bus *usb_bus = rh_usb_dev-bus;
-   struct usb_hcd *usb_hcd;
+   struct usb_hcd *hcd;
 
-   usb_hcd = bus_to_hcd(usb_bus);
+   hcd = bus_to_hcd(usb_bus);
result = sscanf(buf, %u\n, val);
if (result == 1) {
-   usb_hcd-authorized_default = val ? 1 : 0;
+   if (val)
+   set_bit(HCD_FLAG_DEV_AUTHORIZED, hcd-flags);
+   else
+   clear_bit(HCD_FLAG_DEV_AUTHORIZED, hcd-flags);
+
result = size;
} else {
result = -EINVAL;
@@ -2720,10 +2724,17 @@ int usb_add_hcd(struct usb_hcd *hcd,
dev_info(hcd-self.controller, %s\n, hcd-product_desc);
 
/* Keep old behaviour if authorized_default is not in [0, 1]. */
-   if (authorized_default  0 || authorized_default  1)
-   hcd-authorized_default = hcd-wireless ? 0 : 1;
-   else
-   hcd-authorized_default = authorized_default;
+   if (authorized_default  0 || authorized_default  1) {
+   if (hcd-wireless)
+   clear_bit(HCD_FLAG_DEV_AUTHORIZED, hcd-flags);
+   else
+   set_bit(HCD_FLAG_DEV_AUTHORIZED, hcd-flags);
+   } else {
+   if (authorized_default)
+   set_bit(HCD_FLAG_DEV_AUTHORIZED, hcd-flags);
+   else
+   clear_bit(HCD_FLAG_DEV_AUTHORIZED, hcd-flags);
+   }
set_bit(HCD_FLAG_HW_ACCESSIBLE, hcd-flags);
 
/* per default all interfaces are authorized */
diff --git a/drivers/usb/core/usb.c b/drivers/usb/core/usb.c
index 8d5b2f4..f8bbd0b 100644
--- a/drivers/usb/core/usb.c
+++ b/drivers/usb/core/usb.c
@@ -510,7 +510,7 @@ struct usb_device *usb_alloc_dev(struct usb_device *parent,
if (root_hub)   /* Root hub always ok [and always wired] */
dev-authorized = 1;
else {
-   dev-authorized = usb_hcd-authorized_default;
+   dev-authorized = !!HCD_DEV_AUTHORIZED(usb_hcd);
dev-wusb = usb_bus_is_wusb(bus) ? 1 : 0;
}
return dev;
diff --git a/include/linux/usb/hcd.h b/include/linux/usb/hcd.h
index e56c6b2..09a51a4 100644
--- a/include/linux/usb/hcd.h
+++ b/include/linux/usb/hcd.h
@@ -58,12 +58,6 @@
  *
  * Since struct usb_bus is so thin, you can't share much code in it.
  * This framework is a layer over that, and should be more sharable.
- *
- * @authorized_default: Specifies if new devices are authorized to
- *  connect by default or they require explicit
- *  user space authorization; this bit is settable
- *  through /sys/class/usb_host/X/authorized_default.
- *  For the rest is RO, so we don't lock to r/w it.
  */
 
 /*-*/
@@ -121,6 +115,7 @@ struct usb_hcd {
 #define HCD_FLAG_RH_RUNNING5   /* root hub is running? */
 #define HCD_FLAG_DEAD  6   /* controller has died? */
 #define HCD_FLAG_INTF_AUTHORIZED   7   /* authorize interfaces? */
+#define HCD_FLAG_DEV_AUTHORIZED8   /* authorize devices? */
 
/* The flags can be tested using these macros; they are likely to
 * be slightly faster than test_bit().
@@ -140,6 +135,14 @@ struct usb_hcd {
 #define HCD_INTF_AUTHORIZED(hcd) \
((hcd)-flags  (1U  HCD_FLAG_INTF_AUTHORIZED))
 
+   /*
+* Specifies if devices are authorized by default
+* or they require explicit user space authorization; this bit is
+* settable through /sys/class/usb_host/X/authorized_default
+*/
+#define HCD_DEV_AUTHORIZED(hcd) \
+   ((hcd)-flags  (1U  

[PATCH v9 4/7] usb: interface authorization: Introduces the USB interface authorization

2015-08-08 Thread Stefan Koch
The kernel supports the device authorization because of wireless USB.
These is usable for wired USB devices, too.
These new interface authorization allows to enable or disable
individual interfaces instead a whole device.

If a deauthorized interface will be authorized so the driver probing must
be triggered manually by writing INTERFACE to /sys/bus/usb/drivers_probe

Signed-off-by: Stefan Koch sk...@suse.de
---
 drivers/usb/core/message.c | 38 ++
 drivers/usb/core/usb.h |  2 ++
 2 files changed, 40 insertions(+)

diff --git a/drivers/usb/core/message.c b/drivers/usb/core/message.c
index 3d25d89..c090f50 100644
--- a/drivers/usb/core/message.c
+++ b/drivers/usb/core/message.c
@@ -1555,6 +1555,44 @@ static void usb_release_interface(struct device *dev)
kfree(intf);
 }
 
+/*
+ * usb_deauthorize_interface - deauthorize an USB interface
+ *
+ * @intf: USB interface structure
+ */
+void usb_deauthorize_interface(struct usb_interface *intf)
+{
+   struct device *dev = intf-dev;
+
+   device_lock(dev-parent);
+
+   if (intf-authorized) {
+   device_lock(dev);
+   intf-authorized = 0;
+   device_unlock(dev);
+
+   usb_forced_unbind_intf(intf);
+   }
+
+   device_unlock(dev-parent);
+}
+
+/*
+ * usb_authorize_interface - authorize an USB interface
+ *
+ * @intf: USB interface structure
+ */
+void usb_authorize_interface(struct usb_interface *intf)
+{
+   struct device *dev = intf-dev;
+
+   if (!intf-authorized) {
+   device_lock(dev);
+   intf-authorized = 1; /* authorize interface */
+   device_unlock(dev);
+   }
+}
+
 static int usb_if_uevent(struct device *dev, struct kobj_uevent_env *env)
 {
struct usb_device *usb_dev;
diff --git a/drivers/usb/core/usb.h b/drivers/usb/core/usb.h
index 457255a..05b5e17 100644
--- a/drivers/usb/core/usb.h
+++ b/drivers/usb/core/usb.h
@@ -27,6 +27,8 @@ extern void usb_release_interface_cache(struct kref *ref);
 extern void usb_disable_device(struct usb_device *dev, int skip_ep0);
 extern int usb_deauthorize_device(struct usb_device *);
 extern int usb_authorize_device(struct usb_device *);
+extern void usb_deauthorize_interface(struct usb_interface *);
+extern void usb_authorize_interface(struct usb_interface *);
 extern void usb_detect_quirks(struct usb_device *udev);
 extern void usb_detect_interface_quirks(struct usb_device *udev);
 extern int usb_remove_device(struct usb_device *udev);
-- 
2.1.4

--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v9 6/7] usb: interface authorization: Documentation part

2015-08-08 Thread Stefan Koch
This part adds the documentation for the interface authorization.

Signed-off-by: Stefan Koch sk...@suse.de
---
 Documentation/ABI/testing/sysfs-bus-usb | 22 +
 Documentation/usb/authorization.txt | 34 +
 2 files changed, 56 insertions(+)

diff --git a/Documentation/ABI/testing/sysfs-bus-usb 
b/Documentation/ABI/testing/sysfs-bus-usb
index e5cc763..ca8c8d3 100644
--- a/Documentation/ABI/testing/sysfs-bus-usb
+++ b/Documentation/ABI/testing/sysfs-bus-usb
@@ -1,3 +1,25 @@
+What:  /sys/bus/usb/devices/INTERFACE/authorized
+Date:  June 2015
+KernelVersion: 4.2
+Description:
+   This allows to authorize (1) or deauthorize (0)
+   individual interfaces instead a whole device
+   in contrast to the device authorization.
+   If a deauthorized interface will be authorized
+   so the driver probing must be triggered manually
+   by writing INTERFACE to /sys/bus/usb/drivers_probe
+   This allows to avoid side-effects with drivers
+   that need multiple interfaces.
+   A deauthorized interface cannot be probed or claimed.
+
+What:  /sys/bus/usb/devices/usbX/interface_authorized_default
+Date:  June 2015
+KernelVersion: 4.2
+Description:
+   This is used as default value that determines
+   if interfaces would authorized per default.
+   The value can be 1 or 0. It is per default 1.
+
 What:  /sys/bus/usb/device/.../authorized
 Date:  July 2008
 KernelVersion: 2.6.26
diff --git a/Documentation/usb/authorization.txt 
b/Documentation/usb/authorization.txt
index c069b68..020cec5 100644
--- a/Documentation/usb/authorization.txt
+++ b/Documentation/usb/authorization.txt
@@ -3,6 +3,9 @@ Authorizing (or not) your USB devices to connect to the system
 
 (C) 2007 Inaky Perez-Gonzalez in...@linux.intel.com Intel Corporation
 
+Interface authorization part:
+   (C) 2015 Stefan Koch sk...@suse.de SUSE LLC
+
 This feature allows you to control if a USB device can be used (or
 not) in a system. This feature will allow you to implement a lock-down
 of USB devices, fully controlled by user space.
@@ -90,3 +93,34 @@ etc, but you get the idea. Anybody with access to a device 
gadget kit
 can fake descriptors and device info. Don't trust that. You are
 welcome.
 
+
+Interface authorization
+---
+There is a similar approach to allow or deny specific USB interfaces.
+That allows to block only a subset of an USB device.
+
+Authorize an interface:
+$ echo 1  /sys/bus/usb/devices/INTERFACE/authorized
+
+Deauthorize an interface:
+$ echo 0  /sys/bus/usb/devices/INTERFACE/authorized
+
+The default value for new interfaces
+on a particular USB bus can be changed, too.
+
+Allow interfaces per default:
+$ echo 1  /sys/bus/usb/devices/usbX/interface_authorized_default
+
+Deny interfaces per default:
+$ echo 0  /sys/bus/usb/devices/usbX/interface_authorized_default
+
+Per default the interface_authorized_default bit is 1.
+So all interfaces would authorized per default.
+
+Note:
+If a deauthorized interface will be authorized so the driver probing must
+be triggered manually by writing INTERFACE to /sys/bus/usb/drivers_probe
+
+For drivers that need multiple interfaces all needed interfaces should be
+authroized first. After that the drivers should be probed.
+This avoids side effects.
-- 
2.1.4

--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v9 5/7] usb: interface authorization: SysFS part of USB interface authorization

2015-08-08 Thread Stefan Koch
This introduces an attribute for each interface to
authorize (1) or deauthorize (0) it:
/sys/bus/usb/devices/INTERFACE/authorized

Signed-off-by: Stefan Koch sk...@suse.de
---
 drivers/usb/core/sysfs.c | 36 
 1 file changed, 36 insertions(+)

diff --git a/drivers/usb/core/sysfs.c b/drivers/usb/core/sysfs.c
index d269738..3ddaada 100644
--- a/drivers/usb/core/sysfs.c
+++ b/drivers/usb/core/sysfs.c
@@ -926,6 +926,41 @@ static ssize_t supports_autosuspend_show(struct device 
*dev,
 }
 static DEVICE_ATTR_RO(supports_autosuspend);
 
+/*
+ * interface_authorized_show - show authorization status of an USB interface
+ * 1 is authorized, 0 is deauthorized
+ */
+static ssize_t interface_authorized_show(struct device *dev,
+   struct device_attribute *attr, char *buf)
+{
+   struct usb_interface *intf = to_usb_interface(dev);
+
+   return sprintf(buf, %u\n, intf-authorized);
+}
+
+/*
+ * interface_authorized_store - authorize or deauthorize an USB interface
+ */
+static ssize_t interface_authorized_store(struct device *dev,
+   struct device_attribute *attr, const char *buf, size_t count)
+{
+   struct usb_interface *intf = to_usb_interface(dev);
+   bool val;
+
+   if (strtobool(buf, val) != 0)
+   return -EINVAL;
+
+   if (val)
+   usb_authorize_interface(intf);
+   else
+   usb_deauthorize_interface(intf);
+
+   return count;
+}
+static struct device_attribute dev_attr_interface_authorized =
+   __ATTR(authorized, S_IRUGO | S_IWUSR,
+   interface_authorized_show, interface_authorized_store);
+
 static struct attribute *intf_attrs[] = {
dev_attr_bInterfaceNumber.attr,
dev_attr_bAlternateSetting.attr,
@@ -935,6 +970,7 @@ static struct attribute *intf_attrs[] = {
dev_attr_bInterfaceProtocol.attr,
dev_attr_modalias.attr,
dev_attr_supports_autosuspend.attr,
+   dev_attr_interface_authorized.attr,
NULL,
 };
 static struct attribute_group intf_attr_grp = {
-- 
2.1.4

--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v9 6/7] usb: interface authorization: Documentation part

2015-08-08 Thread Sergei Shtylyov

Hello.

On 8/8/2015 12:32 PM, Stefan Koch wrote:


This part adds the documentation for the interface authorization.



Signed-off-by: Stefan Koch sk...@suse.de
---
  Documentation/ABI/testing/sysfs-bus-usb | 22 +
  Documentation/usb/authorization.txt | 34 +
  2 files changed, 56 insertions(+)



diff --git a/Documentation/ABI/testing/sysfs-bus-usb 
b/Documentation/ABI/testing/sysfs-bus-usb
index e5cc763..ca8c8d3 100644
--- a/Documentation/ABI/testing/sysfs-bus-usb
+++ b/Documentation/ABI/testing/sysfs-bus-usb
@@ -1,3 +1,25 @@
+What:  /sys/bus/usb/devices/INTERFACE/authorized
+Date:  June 2015
+KernelVersion: 4.2
+Description:
+   This allows to authorize (1) or deauthorize (0)
+   individual interfaces instead a whole device
+   in contrast to the device authorization.
+   If a deauthorized interface will be authorized
+   so the driver probing must be triggered manually
+   by writing INTERFACE to /sys/bus/usb/drivers_probe
+   This allows to avoid side-effects with drivers
+   that need multiple interfaces.
+   A deauthorized interface cannot be probed or claimed.
+
+What:  /sys/bus/usb/devices/usbX/interface_authorized_default
+Date:  June 2015
+KernelVersion: 4.2
+Description:
+   This is used as default value that determines
+   if interfaces would authorized per default.


   Would be authorized by default?


+   The value can be 1 or 0. It is per default 1.


   s/per/by/?

[...]

diff --git a/Documentation/usb/authorization.txt 
b/Documentation/usb/authorization.txt
index c069b68..020cec5 100644
--- a/Documentation/usb/authorization.txt
+++ b/Documentation/usb/authorization.txt

[...]

@@ -90,3 +93,34 @@ etc, but you get the idea. Anybody with access to a device 
gadget kit
  can fake descriptors and device info. Don't trust that. You are
  welcome.

+
+Interface authorization
+---
+There is a similar approach to allow or deny specific USB interfaces.
+That allows to block only a subset of an USB device.
+
+Authorize an interface:
+$ echo 1  /sys/bus/usb/devices/INTERFACE/authorized
+
+Deauthorize an interface:
+$ echo 0  /sys/bus/usb/devices/INTERFACE/authorized
+
+The default value for new interfaces
+on a particular USB bus can be changed, too.
+
+Allow interfaces per default:
+$ echo 1  /sys/bus/usb/devices/usbX/interface_authorized_default
+
+Deny interfaces per default:
+$ echo 0  /sys/bus/usb/devices/usbX/interface_authorized_default
+
+Per default the interface_authorized_default bit is 1.


   S/Per/By/?


+So all interfaces would authorized per default.


   Likewise...

[...]

MBR, Sergei

--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v4 2/2] usb: Add support for ACPI identification to xhci-platform

2015-08-08 Thread Greg KH
On Sat, Aug 08, 2015 at 07:43:40AM +0200, Javier Martinez Canillas wrote:
 Hello Greg,
 
 On Sat, Aug 8, 2015 at 3:29 AM, Greg KH gre...@linuxfoundation.org wrote:
  On Fri, Aug 07, 2015 at 06:03:36PM -0700, Duc Dang wrote:
  Provide the methods to let ACPI identify the need to use
  xhci-platform. Change the Kconfig files so the
  xhci-plat.o file is selectable during kernel config.
 
  This has been tested on an ARM64 machine with platform XHCI, an
  x86_64 machine with XHCI, and an x86_64 machine without XHCI.
  There were no regressions or error messages on the machines
  without platform XHCI.
 
  [dhdang: regenerate the patch over 4.2-rc5]
  Signed-off-by: Mark Langsdorf mlang...@redhat.com
  Signed-off-by: Duc Dang dhd...@apm.com
 
  ---
  Changes from v3:
Regenerate the patch over 4.2-rc5
No code change
 
  Changes from v2
Replaced tristate with a boolean as the driver doesn't
compile as a module
Correct --help-- to ---help---
 
  Changes from v1
Renamed from add support for APM X-Gene to xhci-platform
Removed changes to arm64/Kconfig
Made CONFIG_USB_XHCI_PLATFORM a user selectable config option
 
   drivers/usb/host/Kconfig |  7 ++-
   drivers/usb/host/xhci-plat.c | 11 +++
   2 files changed, 17 insertions(+), 1 deletion(-)
 
  diff --git a/drivers/usb/host/Kconfig b/drivers/usb/host/Kconfig
  index 8afc3c1..96231ee 100644
  --- a/drivers/usb/host/Kconfig
  +++ b/drivers/usb/host/Kconfig
  @@ -32,7 +32,12 @@ config USB_XHCI_PCI
  default y
 
   config USB_XHCI_PLATFORM
  - tristate
  + tristate xHCI platform driver support
  + ---help---
  +   Say 'Y' to enable the support for the xHCI host controller
  +   as a platform device. Many ARM SoCs provide USB this way.
  +
  +   If unsure, say 'Y'.
 
   config USB_XHCI_MVEBU
tristate xHCI support for Marvell Armada 375/38x
  diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c
  index 5d03f8b..14b40d2 100644
  --- a/drivers/usb/host/xhci-plat.c
  +++ b/drivers/usb/host/xhci-plat.c
  @@ -19,6 +19,7 @@
   #include linux/usb/phy.h
   #include linux/slab.h
   #include linux/usb/xhci_pdriver.h
  +#include linux/acpi.h
 
   #include xhci.h
   #include xhci-mvebu.h
  @@ -262,6 +263,15 @@ static const struct of_device_id usb_xhci_of_match[] 
  = {
   MODULE_DEVICE_TABLE(of, usb_xhci_of_match);
   #endif
 
  +#ifdef CONFIG_ACPI
 
  You shoudn't need this #ifdef, right?
 
 
 Why it is not needed?

Why is it needed?

 The driver does .acpi_match_table = ACPI_PTR(usb_xhci_acpi_match) and
 ACPI_PTR() is NULL if CONFIG_ACPI is not enabled. Which can happen
 AFAIU since the driver also supports OF. So without the #ifdef guards,
 .acpi_match_table = NULL and the struct acpi_device_id
 usb_xhci_acpi_match[] will be built but not used.

Which is just fine, right?

 Or am I missing something?

Don't put #ifdef in .c files if at all possible is the kernel style
rules.

thanks,

greg k-h
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v4 2/2] usb: Add support for ACPI identification to xhci-platform

2015-08-08 Thread Duc Dang
On Sat, Aug 8, 2015 at 8:37 AM, Greg KH gre...@linuxfoundation.org wrote:
 On Sat, Aug 08, 2015 at 07:43:40AM +0200, Javier Martinez Canillas wrote:
 Hello Greg,

 On Sat, Aug 8, 2015 at 3:29 AM, Greg KH gre...@linuxfoundation.org wrote:
  On Fri, Aug 07, 2015 at 06:03:36PM -0700, Duc Dang wrote:
  Provide the methods to let ACPI identify the need to use
  xhci-platform. Change the Kconfig files so the
  xhci-plat.o file is selectable during kernel config.
 
  This has been tested on an ARM64 machine with platform XHCI, an
  x86_64 machine with XHCI, and an x86_64 machine without XHCI.
  There were no regressions or error messages on the machines
  without platform XHCI.
 
  [dhdang: regenerate the patch over 4.2-rc5]
  Signed-off-by: Mark Langsdorf mlang...@redhat.com
  Signed-off-by: Duc Dang dhd...@apm.com
 
  ---
  Changes from v3:
Regenerate the patch over 4.2-rc5
No code change
 
  Changes from v2
Replaced tristate with a boolean as the driver doesn't
compile as a module
Correct --help-- to ---help---
 
  Changes from v1
Renamed from add support for APM X-Gene to xhci-platform
Removed changes to arm64/Kconfig
Made CONFIG_USB_XHCI_PLATFORM a user selectable config option
 
   drivers/usb/host/Kconfig |  7 ++-
   drivers/usb/host/xhci-plat.c | 11 +++
   2 files changed, 17 insertions(+), 1 deletion(-)
 
  diff --git a/drivers/usb/host/Kconfig b/drivers/usb/host/Kconfig
  index 8afc3c1..96231ee 100644
  --- a/drivers/usb/host/Kconfig
  +++ b/drivers/usb/host/Kconfig
  @@ -32,7 +32,12 @@ config USB_XHCI_PCI
  default y
 
   config USB_XHCI_PLATFORM
  - tristate
  + tristate xHCI platform driver support
  + ---help---
  +   Say 'Y' to enable the support for the xHCI host controller
  +   as a platform device. Many ARM SoCs provide USB this way.
  +
  +   If unsure, say 'Y'.
 
   config USB_XHCI_MVEBU
tristate xHCI support for Marvell Armada 375/38x
  diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c
  index 5d03f8b..14b40d2 100644
  --- a/drivers/usb/host/xhci-plat.c
  +++ b/drivers/usb/host/xhci-plat.c
  @@ -19,6 +19,7 @@
   #include linux/usb/phy.h
   #include linux/slab.h
   #include linux/usb/xhci_pdriver.h
  +#include linux/acpi.h
 
   #include xhci.h
   #include xhci-mvebu.h
  @@ -262,6 +263,15 @@ static const struct of_device_id usb_xhci_of_match[] 
  = {
   MODULE_DEVICE_TABLE(of, usb_xhci_of_match);
   #endif
 
  +#ifdef CONFIG_ACPI
 
  You shoudn't need this #ifdef, right?
 

 Why it is not needed?

 Why is it needed?

 The driver does .acpi_match_table = ACPI_PTR(usb_xhci_acpi_match) and
 ACPI_PTR() is NULL if CONFIG_ACPI is not enabled. Which can happen
 AFAIU since the driver also supports OF. So without the #ifdef guards,
 .acpi_match_table = NULL and the struct acpi_device_id
 usb_xhci_acpi_match[] will be built but not used.

 Which is just fine, right?

 Or am I missing something?

 Don't put #ifdef in .c files if at all possible is the kernel style
 rules.

I tested booting with both device tree and ACPI with the new code that
has #ifdef CONFIG_ACPI removed and USB works fine with my X-Gene Arm64
platform.


 thanks,

 greg k-h



-- 
Regards,
Duc Dang.
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v5 1/2] usb: make xhci platform driver use 64 bit or 32 bit DMA

2015-08-08 Thread Duc Dang
On Sat, Aug 8, 2015 at 2:22 AM, Russell King - ARM Linux
li...@arm.linux.org.uk wrote:
 On Fri, Aug 07, 2015 at 08:18:48PM -0700, Duc Dang wrote:
 diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c
 index 890ad9d..5d03f8b 100644
 --- a/drivers/usb/host/xhci-plat.c
 +++ b/drivers/usb/host/xhci-plat.c
 @@ -93,14 +93,14 @@ static int xhci_plat_probe(struct platform_device *pdev)
   if (irq  0)
   return -ENODEV;

 - /* Initialize dma_mask and coherent_dma_mask to 32-bits */
 - ret = dma_set_coherent_mask(pdev-dev, DMA_BIT_MASK(32));
 - if (ret)
 - return ret;
 - if (!pdev-dev.dma_mask)
 - pdev-dev.dma_mask = pdev-dev.coherent_dma_mask;
 - else
 - dma_set_mask(pdev-dev, DMA_BIT_MASK(32));
 + /* Try setting the coherent_dma_mask to 64 bits, then try 32 bits */
 + ret = dma_set_mask_and_coherent(pdev-dev, DMA_BIT_MASK(64));
 + if (ret) {
 + ret = dma_set_mask_and_coherent(pdev-dev, DMA_BIT_MASK(32));
 + if (ret)
 + return ret;
 + }

 Note that dma_set_mask_and_coherent() and the original code are not
 equivalent because of this:

 if (!pdev-dev.dma_mask)
 pdev-dev.dma_mask = pdev-dev.coherent_dma_mask;

 If we know that pdev-dev.dma_mask will always be initialised at this
 point, then the above change is fine.  If not, it's introducing a
 regression - dma_set_mask_and_coherent() will fail if pdev-dev.dma_mask
 is NULL (depending on the architectures implementation of dma_set_mask()).

 Prefixing the above change with the two lines I mention above would
 ensure equivalent behaviour.  Even if we do want to get rid of this,
 I'd advise to do it as a separate patch after this change, which can
 be independently reverted if there's problems with its removal.

Hi Russell,

I will add the 2 lines you mentioned back to next version of the
patch. It is safer to do it that way as I do not see
pdev-dev.dma_mask gets initialized before the call
dma_set_mask_and_coherent  inside this xhci_plat.c file.

 --
 FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up
 according to speedtest.net.

-- 
Regards,
Duc Dang.
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v4 2/2] usb: Add support for ACPI identification to xhci-platform

2015-08-08 Thread Javier Martinez Canillas
Hello Greg,

On Sat, Aug 8, 2015 at 5:37 PM, Greg KH gre...@linuxfoundation.org wrote:
 On Sat, Aug 08, 2015 at 07:43:40AM +0200, Javier Martinez Canillas wrote:
 Hello Greg,

 On Sat, Aug 8, 2015 at 3:29 AM, Greg KH gre...@linuxfoundation.org wrote:
  On Fri, Aug 07, 2015 at 06:03:36PM -0700, Duc Dang wrote:
  Provide the methods to let ACPI identify the need to use
  xhci-platform. Change the Kconfig files so the
  xhci-plat.o file is selectable during kernel config.
 
  This has been tested on an ARM64 machine with platform XHCI, an
  x86_64 machine with XHCI, and an x86_64 machine without XHCI.
  There were no regressions or error messages on the machines
  without platform XHCI.
 
  [dhdang: regenerate the patch over 4.2-rc5]
  Signed-off-by: Mark Langsdorf mlang...@redhat.com
  Signed-off-by: Duc Dang dhd...@apm.com
 
  ---
  Changes from v3:
Regenerate the patch over 4.2-rc5
No code change
 
  Changes from v2
Replaced tristate with a boolean as the driver doesn't
compile as a module
Correct --help-- to ---help---
 
  Changes from v1
Renamed from add support for APM X-Gene to xhci-platform
Removed changes to arm64/Kconfig
Made CONFIG_USB_XHCI_PLATFORM a user selectable config option
 
   drivers/usb/host/Kconfig |  7 ++-
   drivers/usb/host/xhci-plat.c | 11 +++
   2 files changed, 17 insertions(+), 1 deletion(-)
 
  diff --git a/drivers/usb/host/Kconfig b/drivers/usb/host/Kconfig
  index 8afc3c1..96231ee 100644
  --- a/drivers/usb/host/Kconfig
  +++ b/drivers/usb/host/Kconfig
  @@ -32,7 +32,12 @@ config USB_XHCI_PCI
  default y
 
   config USB_XHCI_PLATFORM
  - tristate
  + tristate xHCI platform driver support
  + ---help---
  +   Say 'Y' to enable the support for the xHCI host controller
  +   as a platform device. Many ARM SoCs provide USB this way.
  +
  +   If unsure, say 'Y'.
 
   config USB_XHCI_MVEBU
tristate xHCI support for Marvell Armada 375/38x
  diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c
  index 5d03f8b..14b40d2 100644
  --- a/drivers/usb/host/xhci-plat.c
  +++ b/drivers/usb/host/xhci-plat.c
  @@ -19,6 +19,7 @@
   #include linux/usb/phy.h
   #include linux/slab.h
   #include linux/usb/xhci_pdriver.h
  +#include linux/acpi.h
 
   #include xhci.h
   #include xhci-mvebu.h
  @@ -262,6 +263,15 @@ static const struct of_device_id usb_xhci_of_match[] 
  = {
   MODULE_DEVICE_TABLE(of, usb_xhci_of_match);
   #endif
 
  +#ifdef CONFIG_ACPI
 
  You shoudn't need this #ifdef, right?
 

 Why it is not needed?

 Why is it needed?


As explained, to have avoid having an unused variable.

 The driver does .acpi_match_table = ACPI_PTR(usb_xhci_acpi_match) and
 ACPI_PTR() is NULL if CONFIG_ACPI is not enabled. Which can happen
 AFAIU since the driver also supports OF. So without the #ifdef guards,
 .acpi_match_table = NULL and the struct acpi_device_id
 usb_xhci_acpi_match[] will be built but not used.

 Which is just fine, right?


I've seen people having different opinions about this specific case
(using #ifdef guards for ACPI, OF, etc match tables definition),
that's why I asked.

 Or am I missing something?

 Don't put #ifdef in .c files if at all possible is the kernel style
 rules.


I know but as you said the rule is to not have #ifdef if possible. But
I understand now that for you this case doesn't justify the #ifdefery.

 thanks,

 greg k-h

Best regards,
Javier
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH] phy: lpc18xx-usb-otg: fix clock order in phy init

2015-08-08 Thread Joachim Eastwood
Changing the frequency of the USB clock must be done before the
PLL is powered on (prepared). This matters when the USB clock
is not setup by either boot ROM or boot loader. Reorder the
function calls to adhere to the order noted in the user manual.

Signed-off-by: Joachim Eastwood manab...@gmail.com
---
Hi Kishon,

Here one small fix for lpc18xx-usb-otg which I hope can go in
for 4.3. While not a critical fix it will be needed when I add
support for USB0 clock setup in Linux which I hope to add for
4.4. This patch is prerequisite for that.

Right now USB only works if clocks are setup by either boot ROM
or boot loader.

Patch based on the next branch in your phy tree.

 drivers/phy/phy-lpc18xx-usb-otg.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/phy/phy-lpc18xx-usb-otg.c 
b/drivers/phy/phy-lpc18xx-usb-otg.c
index 3aa8e4de1b03..3b7a71eb5b7e 100644
--- a/drivers/phy/phy-lpc18xx-usb-otg.c
+++ b/drivers/phy/phy-lpc18xx-usb-otg.c
@@ -33,12 +33,12 @@ static int lpc18xx_usb_otg_phy_init(struct phy *phy)
struct lpc18xx_usb_otg_phy *lpc = phy_get_drvdata(phy);
int ret;
 
-   ret = clk_prepare(lpc-clk);
+   /* The PHY must be clocked at 480 MHz */
+   ret = clk_set_rate(lpc-clk, 48000);
if (ret)
return ret;
 
-   /* The PHY must be clocked at 480 MHz */
-   return clk_set_rate(lpc-clk, 48000);
+   return clk_prepare(lpc-clk);
 }
 
 static int lpc18xx_usb_otg_phy_exit(struct phy *phy)
-- 
1.8.0

--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html