Re: [PATCHv2 5/6] base: platform: name the device already during allocation

2014-08-05 Thread Heikki Krogerus 
On Mon, Jul 14, 2014 at 07:55:43AM -0700, Greg Kroah-Hartman wrote:
   diff --git a/drivers/base/platform.c b/drivers/base/platform.c
   index 9e9227e..e856bc4 100644
   --- a/drivers/base/platform.c
   +++ b/drivers/base/platform.c
   @@ -177,11 +177,45 @@ struct platform_object {
 */
void platform_device_put(struct platform_device *pdev)
{
   - if (pdev)
   - put_device(pdev-dev);
   + if (!pdev)
   + return;
   +
   + if (pdev-id_auto) {
   + ida_simple_remove(platform_devid_ida, pdev-id);
   + pdev-id = PLATFORM_DEVID_AUTO;
   + }
   +
   + put_device(pdev-dev);
}
EXPORT_SYMBOL_GPL(platform_device_put);
 
 Why would a single call to this function remove an id?  That seems
 really wrong, you should be able to call get and put on a device
 numerous times, only the last reference should cause the device to be
 cleaned up.
 
 Shouldn't this be in the release function instead?

I'll fix this.

Thanks Greg.

-- 
heikki
--
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: [PATCHv2 5/6] base: platform: name the device already during allocation

2014-08-04 Thread Vivek Gautam 
On Mon, Jul 14, 2014 at 8:25 PM, Greg Kroah-Hartman
gre...@linuxfoundation.org wrote:
 On Mon, Jul 14, 2014 at 11:49:38AM +0530, Kishon Vijay Abraham I wrote:
 Greg,

 On Thursday 05 June 2014 06:22 PM, Heikki Krogerus wrote:
  This allows resources such as GPIOs and clocks, which can be
  matched based on the device name when requested, to be
  assigned even when PLATFORM_DEVID_AUTO is used.

 Any comments on this patch?

 I thought I rejected it in the past, don't have my email archives
 around at the moment, sorry.


  Signed-off-by: Heikki Krogerus heikki.kroge...@linux.intel.com

Are we planning a different approach for this ?
Since we are basing the phy-calibration patches for phy-exynos5-usbdrd
on this series
and we need to register the phy lookup table in DWC3, and get the PHYs
in xhci-plat.

The following patch in this series (which is based on this change)
help us achieve that :
[PATCHv2 6/6] usb: dwc3: host: convey the PHYs to xhci
(https://lkml.org/lkml/2014/6/5/585)

  Cc: Greg Kroah-Hartman gre...@linuxfoundation.org
  ---
   drivers/base/platform.c | 77 
  ++---
   1 file changed, 47 insertions(+), 30 deletions(-)
 
  diff --git a/drivers/base/platform.c b/drivers/base/platform.c
  index 9e9227e..e856bc4 100644
  --- a/drivers/base/platform.c
  +++ b/drivers/base/platform.c
  @@ -177,11 +177,45 @@ struct platform_object {
*/
   void platform_device_put(struct platform_device *pdev)
   {
  -   if (pdev)
  -   put_device(pdev-dev);
  +   if (!pdev)
  +   return;
  +
  +   if (pdev-id_auto) {
  +   ida_simple_remove(platform_devid_ida, pdev-id);
  +   pdev-id = PLATFORM_DEVID_AUTO;
  +   }
  +
  +   put_device(pdev-dev);
   }
   EXPORT_SYMBOL_GPL(platform_device_put);

 Why would a single call to this function remove an id?  That seems
 really wrong, you should be able to call get and put on a device
 numerous times, only the last reference should cause the device to be
 cleaned up.

 Shouldn't this be in the release function instead?

 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



-- 
Best Regards
Vivek Gautam
Samsung RD Institute, Bangalore
India
--
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: [PATCHv2 5/6] base: platform: name the device already during allocation

2014-07-14 Thread Kishon Vijay Abraham I 
Greg,

On Thursday 05 June 2014 06:22 PM, Heikki Krogerus wrote:
 This allows resources such as GPIOs and clocks, which can be
 matched based on the device name when requested, to be
 assigned even when PLATFORM_DEVID_AUTO is used.

Any comments on this patch?

Thanks
Kishon
 
 Signed-off-by: Heikki Krogerus heikki.kroge...@linux.intel.com
 Cc: Greg Kroah-Hartman gre...@linuxfoundation.org
 ---
  drivers/base/platform.c | 77 
 ++---
  1 file changed, 47 insertions(+), 30 deletions(-)
 
 diff --git a/drivers/base/platform.c b/drivers/base/platform.c
 index 9e9227e..e856bc4 100644
 --- a/drivers/base/platform.c
 +++ b/drivers/base/platform.c
 @@ -177,11 +177,45 @@ struct platform_object {
   */
  void platform_device_put(struct platform_device *pdev)
  {
 - if (pdev)
 - put_device(pdev-dev);
 + if (!pdev)
 + return;
 +
 + if (pdev-id_auto) {
 + ida_simple_remove(platform_devid_ida, pdev-id);
 + pdev-id = PLATFORM_DEVID_AUTO;
 + }
 +
 + put_device(pdev-dev);
  }
  EXPORT_SYMBOL_GPL(platform_device_put);
  
 +static int pdev_set_name(struct platform_device *pdev)
 +{
 + int ret;
 +
 + switch (pdev-id) {
 + default:
 + return dev_set_name(pdev-dev, %s.%d, pdev-name,  pdev-id);
 + case PLATFORM_DEVID_NONE:
 + return dev_set_name(pdev-dev, %s, pdev-name);
 + case PLATFORM_DEVID_AUTO:
 + /*
 +  * Automatically allocated device ID. We mark it as such so
 +  * that we remember it must be freed, and we append a suffix
 +  * to avoid namespace collision with explicit IDs.
 +  */
 + ret = ida_simple_get(platform_devid_ida, 0, 0, GFP_KERNEL);
 + if (ret  0)
 + return ret;
 + pdev-id = ret;
 + pdev-id_auto = true;
 + return dev_set_name(pdev-dev, %s.%d.auto, pdev-name,
 + pdev-id);
 + }
 +
 + return 0;
 +}
 +
  static void platform_device_release(struct device *dev)
  {
   struct platform_object *pa = container_of(dev, struct platform_object,
 @@ -214,6 +248,10 @@ struct platform_device *platform_device_alloc(const char 
 *name, int id)
   device_initialize(pa-pdev.dev);
   pa-pdev.dev.release = platform_device_release;
   arch_setup_pdev_archdata(pa-pdev);
 + if (pdev_set_name(pa-pdev)) {
 + kfree(pa);
 + return NULL;
 + }
   }
  
   return pa ? pa-pdev : NULL;
 @@ -294,28 +332,6 @@ int platform_device_add(struct platform_device *pdev)
  
   pdev-dev.bus = platform_bus_type;
  
 - switch (pdev-id) {
 - default:
 - dev_set_name(pdev-dev, %s.%d, pdev-name,  pdev-id);
 - break;
 - case PLATFORM_DEVID_NONE:
 - dev_set_name(pdev-dev, %s, pdev-name);
 - break;
 - case PLATFORM_DEVID_AUTO:
 - /*
 -  * Automatically allocated device ID. We mark it as such so
 -  * that we remember it must be freed, and we append a suffix
 -  * to avoid namespace collision with explicit IDs.
 -  */
 - ret = ida_simple_get(platform_devid_ida, 0, 0, GFP_KERNEL);
 - if (ret  0)
 - goto err_out;
 - pdev-id = ret;
 - pdev-id_auto = true;
 - dev_set_name(pdev-dev, %s.%d.auto, pdev-name, pdev-id);
 - break;
 - }
 -
   for (i = 0; i  pdev-num_resources; i++) {
   struct resource *p, *r = pdev-resource[i];
  
 @@ -358,7 +374,6 @@ int platform_device_add(struct platform_device *pdev)
   release_resource(r);
   }
  
 - err_out:
   return ret;
  }
  EXPORT_SYMBOL_GPL(platform_device_add);
 @@ -378,11 +393,6 @@ void platform_device_del(struct platform_device *pdev)
   if (pdev) {
   device_del(pdev-dev);
  
 - if (pdev-id_auto) {
 - ida_simple_remove(platform_devid_ida, pdev-id);
 - pdev-id = PLATFORM_DEVID_AUTO;
 - }
 -
   for (i = 0; i  pdev-num_resources; i++) {
   struct resource *r = pdev-resource[i];
   unsigned long type = resource_type(r);
 @@ -400,8 +410,15 @@ EXPORT_SYMBOL_GPL(platform_device_del);
   */
  int platform_device_register(struct platform_device *pdev)
  {
 + int ret;
 +
   device_initialize(pdev-dev);
   arch_setup_pdev_archdata(pdev);
 +
 + ret = pdev_set_name(pdev);
 + if (ret)
 + return ret;
 +
   return platform_device_add(pdev);
  }
  EXPORT_SYMBOL_GPL(platform_device_register);
 
--
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

Re: [PATCHv2 5/6] base: platform: name the device already during allocation

2014-07-14 Thread Greg Kroah-Hartman 
On Mon, Jul 14, 2014 at 11:49:38AM +0530, Kishon Vijay Abraham I wrote:
 Greg,
 
 On Thursday 05 June 2014 06:22 PM, Heikki Krogerus wrote:
  This allows resources such as GPIOs and clocks, which can be
  matched based on the device name when requested, to be
  assigned even when PLATFORM_DEVID_AUTO is used.
 
 Any comments on this patch?

I thought I rejected it in the past, don't have my email archives
around at the moment, sorry.


  Signed-off-by: Heikki Krogerus heikki.kroge...@linux.intel.com
  Cc: Greg Kroah-Hartman gre...@linuxfoundation.org
  ---
   drivers/base/platform.c | 77 
  ++---
   1 file changed, 47 insertions(+), 30 deletions(-)
  
  diff --git a/drivers/base/platform.c b/drivers/base/platform.c
  index 9e9227e..e856bc4 100644
  --- a/drivers/base/platform.c
  +++ b/drivers/base/platform.c
  @@ -177,11 +177,45 @@ struct platform_object {
*/
   void platform_device_put(struct platform_device *pdev)
   {
  -   if (pdev)
  -   put_device(pdev-dev);
  +   if (!pdev)
  +   return;
  +
  +   if (pdev-id_auto) {
  +   ida_simple_remove(platform_devid_ida, pdev-id);
  +   pdev-id = PLATFORM_DEVID_AUTO;
  +   }
  +
  +   put_device(pdev-dev);
   }
   EXPORT_SYMBOL_GPL(platform_device_put);

Why would a single call to this function remove an id?  That seems
really wrong, you should be able to call get and put on a device
numerous times, only the last reference should cause the device to be
cleaned up.

Shouldn't this be in the release function instead?

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

[PATCHv2 5/6] base: platform: name the device already during allocation

2014-06-05 Thread Heikki Krogerus 
This allows resources such as GPIOs and clocks, which can be
matched based on the device name when requested, to be
assigned even when PLATFORM_DEVID_AUTO is used.

Signed-off-by: Heikki Krogerus heikki.kroge...@linux.intel.com
Cc: Greg Kroah-Hartman gre...@linuxfoundation.org
---
 drivers/base/platform.c | 77 ++---
 1 file changed, 47 insertions(+), 30 deletions(-)

diff --git a/drivers/base/platform.c b/drivers/base/platform.c
index 9e9227e..e856bc4 100644
--- a/drivers/base/platform.c
+++ b/drivers/base/platform.c
@@ -177,11 +177,45 @@ struct platform_object {
  */
 void platform_device_put(struct platform_device *pdev)
 {
-   if (pdev)
-   put_device(pdev-dev);
+   if (!pdev)
+   return;
+
+   if (pdev-id_auto) {
+   ida_simple_remove(platform_devid_ida, pdev-id);
+   pdev-id = PLATFORM_DEVID_AUTO;
+   }
+
+   put_device(pdev-dev);
 }
 EXPORT_SYMBOL_GPL(platform_device_put);
 
+static int pdev_set_name(struct platform_device *pdev)
+{
+   int ret;
+
+   switch (pdev-id) {
+   default:
+   return dev_set_name(pdev-dev, %s.%d, pdev-name,  pdev-id);
+   case PLATFORM_DEVID_NONE:
+   return dev_set_name(pdev-dev, %s, pdev-name);
+   case PLATFORM_DEVID_AUTO:
+   /*
+* Automatically allocated device ID. We mark it as such so
+* that we remember it must be freed, and we append a suffix
+* to avoid namespace collision with explicit IDs.
+*/
+   ret = ida_simple_get(platform_devid_ida, 0, 0, GFP_KERNEL);
+   if (ret  0)
+   return ret;
+   pdev-id = ret;
+   pdev-id_auto = true;
+   return dev_set_name(pdev-dev, %s.%d.auto, pdev-name,
+   pdev-id);
+   }
+
+   return 0;
+}
+
 static void platform_device_release(struct device *dev)
 {
struct platform_object *pa = container_of(dev, struct platform_object,
@@ -214,6 +248,10 @@ struct platform_device *platform_device_alloc(const char 
*name, int id)
device_initialize(pa-pdev.dev);
pa-pdev.dev.release = platform_device_release;
arch_setup_pdev_archdata(pa-pdev);
+   if (pdev_set_name(pa-pdev)) {
+   kfree(pa);
+   return NULL;
+   }
}
 
return pa ? pa-pdev : NULL;
@@ -294,28 +332,6 @@ int platform_device_add(struct platform_device *pdev)
 
pdev-dev.bus = platform_bus_type;
 
-   switch (pdev-id) {
-   default:
-   dev_set_name(pdev-dev, %s.%d, pdev-name,  pdev-id);
-   break;
-   case PLATFORM_DEVID_NONE:
-   dev_set_name(pdev-dev, %s, pdev-name);
-   break;
-   case PLATFORM_DEVID_AUTO:
-   /*
-* Automatically allocated device ID. We mark it as such so
-* that we remember it must be freed, and we append a suffix
-* to avoid namespace collision with explicit IDs.
-*/
-   ret = ida_simple_get(platform_devid_ida, 0, 0, GFP_KERNEL);
-   if (ret  0)
-   goto err_out;
-   pdev-id = ret;
-   pdev-id_auto = true;
-   dev_set_name(pdev-dev, %s.%d.auto, pdev-name, pdev-id);
-   break;
-   }
-
for (i = 0; i  pdev-num_resources; i++) {
struct resource *p, *r = pdev-resource[i];
 
@@ -358,7 +374,6 @@ int platform_device_add(struct platform_device *pdev)
release_resource(r);
}
 
- err_out:
return ret;
 }
 EXPORT_SYMBOL_GPL(platform_device_add);
@@ -378,11 +393,6 @@ void platform_device_del(struct platform_device *pdev)
if (pdev) {
device_del(pdev-dev);
 
-   if (pdev-id_auto) {
-   ida_simple_remove(platform_devid_ida, pdev-id);
-   pdev-id = PLATFORM_DEVID_AUTO;
-   }
-
for (i = 0; i  pdev-num_resources; i++) {
struct resource *r = pdev-resource[i];
unsigned long type = resource_type(r);
@@ -400,8 +410,15 @@ EXPORT_SYMBOL_GPL(platform_device_del);
  */
 int platform_device_register(struct platform_device *pdev)
 {
+   int ret;
+
device_initialize(pdev-dev);
arch_setup_pdev_archdata(pdev);
+
+   ret = pdev_set_name(pdev);
+   if (ret)
+   return ret;
+
return platform_device_add(pdev);
 }
 EXPORT_SYMBOL_GPL(platform_device_register);
-- 
2.0.0.rc4

--
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