Re: [PATCH 2/2] arm: shmobile: lager: enable HS-USB

2014-10-07 Thread Yoshihiro Shimoda
Hello.

(2014/10/07 12:25), Yoshihiro Shimoda wrote:
 Hello.
 
 (2014/10/06 9:59), Yoshihiro Shimoda wrote:
 Hello.

 (2014/10/04 4:50), Sergei Shtylyov wrote:
 On 10/02/2014 12:04 PM, Yoshihiro Shimoda wrote:

 snip 
 +hsusb {
 +  status = okay;
 +  renesas,enable-gpio = gpio5 18 GPIO_ACTIVE_LOW;

 It's certainly active-high.

 Since the current code has the following, we have to set the active_low...
 However, the code is unreadable, I think. So, I will modify the code.

  /* check GPIO determining if USB function should be enabled */
  if (priv-dparam.enable_gpio) {
  gpio_request_one(priv-dparam.enable_gpio, GPIOF_IN, NULL);
  ret = !gpio_get_value(priv-dparam.enable_gpio);
  gpio_free(priv-dparam.enable_gpio);
  if (ret) {
  dev_warn(pdev-dev,
   USB function not selected (GPIO %d)\n,
   priv-dparam.enable_gpio);
  ret = -ENOTSUPP;
  goto probe_end_mod_exit;
  }
  }
 
 I am confusing about the gpio_get_value()...
 In case of ARM, gpio_get_value() will call gpiod_get_raw_value() finally.
 
 gpio_get_value() in arch/arm/include/asm/gpio.h
  -- __gpio_get_value() in include/asm-generic/gpio.h
   -- gpiod_get_raw_value() in drivers/gpio/gpiolib.c
 
 The gpiod_get_raw_value() doesn't care of the GPIO_ACTIVE_{HIGH,LOW}.
 So, should I add gpiod_is_active_low() or someting in the renesas_usbhs 
 driver?
 Or, Do I misunderstand something?

I looked at the gpio-keys driver, and then I understand the renesas_usbhs 
driver should
have the gpio flags from of_get_named_gpio_flags().

So, I will try to modify the renesas_usbhs driver.

Best regards,
Yoshihiro Shimoda

 Best regards,
 Yoshihiro Shimoda
 
 Best regards,
 Yoshihiro Shimoda

 WBR, 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 2/2] arm: shmobile: lager: enable HS-USB

2014-10-07 Thread Sergei Shtylyov

Hello.

On 10/7/2014 4:20 AM, Yoshihiro Shimoda wrote:


Signed-off-by: Yoshihiro Shimoda yoshihiro.shimoda...@renesas.com
---
   arch/arm/boot/dts/r8a7790-lager.dts |5 +
   1 file changed, 5 insertions(+)



diff --git a/arch/arm/boot/dts/r8a7790-lager.dts 
b/arch/arm/boot/dts/r8a7790-lager.dts
index 1698591..4badd0a 100644
--- a/arch/arm/boot/dts/r8a7790-lager.dts
+++ b/arch/arm/boot/dts/r8a7790-lager.dts
@@ -445,3 +445,8 @@
};
};
   };
+
+hsusb {
+   status = okay;
+   renesas,enable-gpio = gpio5 18 GPIO_ACTIVE_LOW;
+};



 Darn, this also misses the PinMux props... I was going to post v2 of these
patches but now would need to add that and retest... :-/



I'm sorry for the my delayed work... I will submit v2 patches today.



By the way, I cannot understand that this also misses the PinMux props.
According to the original legacy code in board-lager.c, it uses gpio5 18:
https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=36be7686caa05334ca8d52df157b373a41d8d9f1


   Right. But there are also USB0 pins.


Best regards,
Yoshihiro Shimoda


WBR, 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 2/2] arm: shmobile: lager: enable HS-USB

2014-10-07 Thread Sergei Shtylyov

On 10/7/2014 7:25 AM, Yoshihiro Shimoda wrote:


Signed-off-by: Yoshihiro Shimoda yoshihiro.shimoda...@renesas.com
---
   arch/arm/boot/dts/r8a7790-lager.dts |5 +
   1 file changed, 5 insertions(+)



diff --git a/arch/arm/boot/dts/r8a7790-lager.dts 
b/arch/arm/boot/dts/r8a7790-lager.dts
index 1698591..4badd0a 100644
--- a/arch/arm/boot/dts/r8a7790-lager.dts
+++ b/arch/arm/boot/dts/r8a7790-lager.dts
@@ -445,3 +445,8 @@
};
};
   };
+
+hsusb {
+   status = okay;
+   renesas,enable-gpio = gpio5 18 GPIO_ACTIVE_LOW;



 It's certainly active-high.



Since the current code has the following, we have to set the active_low...
However, the code is unreadable, I think. So, I will modify the code.

/* check GPIO determining if USB function should be enabled */
if (priv-dparam.enable_gpio) {
gpio_request_one(priv-dparam.enable_gpio, GPIOF_IN, NULL);
ret = !gpio_get_value(priv-dparam.enable_gpio);
gpio_free(priv-dparam.enable_gpio);
if (ret) {
dev_warn(pdev-dev,
 USB function not selected (GPIO %d)\n,
 priv-dparam.enable_gpio);
ret = -ENOTSUPP;
goto probe_end_mod_exit;
}
}



I am confusing about the gpio_get_value()...
In case of ARM, gpio_get_value() will call gpiod_get_raw_value() finally.



gpio_get_value() in arch/arm/include/asm/gpio.h
  -- __gpio_get_value() in include/asm-generic/gpio.h
   -- gpiod_get_raw_value() in drivers/gpio/gpiolib.c



The gpiod_get_raw_value() doesn't care of the GPIO_ACTIVE_{HIGH,LOW}.
So, should I add gpiod_is_active_low() or someting in the renesas_usbhs driver?


   Why do you still think it's active-low?!


Or, Do I misunderstand something?


   Look at the above code again.


Best regards,
Yoshihiro Shimoda


WBR, 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 2/2] arm: shmobile: lager: enable HS-USB

2014-10-07 Thread Yoshihiro Shimoda
Hello.

(2014/10/07 18:46), Sergei Shtylyov wrote:
 On 10/7/2014 7:25 AM, Yoshihiro Shimoda wrote:
 
 Signed-off-by: Yoshihiro Shimoda yoshihiro.shimoda...@renesas.com
 ---
arch/arm/boot/dts/r8a7790-lager.dts |5 +
1 file changed, 5 insertions(+)
 
 diff --git a/arch/arm/boot/dts/r8a7790-lager.dts 
 b/arch/arm/boot/dts/r8a7790-lager.dts
 index 1698591..4badd0a 100644
 --- a/arch/arm/boot/dts/r8a7790-lager.dts
 +++ b/arch/arm/boot/dts/r8a7790-lager.dts
 @@ -445,3 +445,8 @@
};
};
};
 +
 +hsusb {
 +status = okay;
 +renesas,enable-gpio = gpio5 18 GPIO_ACTIVE_LOW;
 
  It's certainly active-high.
 
 Since the current code has the following, we have to set the active_low...
 However, the code is unreadable, I think. So, I will modify the code.

 /* check GPIO determining if USB function should be enabled */
 if (priv-dparam.enable_gpio) {
 gpio_request_one(priv-dparam.enable_gpio, GPIOF_IN, NULL);
 ret = !gpio_get_value(priv-dparam.enable_gpio);
 gpio_free(priv-dparam.enable_gpio);
 if (ret) {
 dev_warn(pdev-dev,
  USB function not selected (GPIO %d)\n,
  priv-dparam.enable_gpio);
 ret = -ENOTSUPP;
 goto probe_end_mod_exit;
 }
 }
 
 I am confusing about the gpio_get_value()...
 In case of ARM, gpio_get_value() will call gpiod_get_raw_value() finally.
 
 gpio_get_value() in arch/arm/include/asm/gpio.h
   -- __gpio_get_value() in include/asm-generic/gpio.h
-- gpiod_get_raw_value() in drivers/gpio/gpiolib.c
 
 The gpiod_get_raw_value() doesn't care of the GPIO_ACTIVE_{HIGH,LOW}.
 So, should I add gpiod_is_active_low() or someting in the renesas_usbhs 
 driver?
 
Why do you still think it's active-low?!
 
 Or, Do I misunderstand something?
 
Look at the above code again.

Thank you for the comment.
Finally, I understood this. I will not modify the driver and
I just change the GPIO_ACTIVE_LOW in the dts file to GPIO_ACTIVE_HIGH
because of your previous comment below:

 'ret' is non-zero here if gpio_get_value() returned 0, so 0 means host
 and 1 means gadget, i.e. GPIO is active-high.

Best regards,
Yoshihiro Shimoda

 Best regards,
 Yoshihiro Shimoda
 
 WBR, 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 2/2] arm: shmobile: lager: enable HS-USB

2014-10-06 Thread Sergei Shtylyov

Hello.

On 10/6/2014 4:59 AM, Yoshihiro Shimoda wrote:


Signed-off-by: Yoshihiro Shimoda yoshihiro.shimoda...@renesas.com
---
   arch/arm/boot/dts/r8a7790-lager.dts |5 +
   1 file changed, 5 insertions(+)



diff --git a/arch/arm/boot/dts/r8a7790-lager.dts 
b/arch/arm/boot/dts/r8a7790-lager.dts
index 1698591..4badd0a 100644
--- a/arch/arm/boot/dts/r8a7790-lager.dts
+++ b/arch/arm/boot/dts/r8a7790-lager.dts
@@ -445,3 +445,8 @@
};
};
   };
+
+hsusb {
+   status = okay;
+   renesas,enable-gpio = gpio5 18 GPIO_ACTIVE_LOW;



 It's certainly active-high.



Since the current code has the following, we have to set the active_low...


   Exactly opposite conclusion follows from this code.


However, the code is unreadable, I think. So, I will modify the code.


   There's no dire need, I think.


/* check GPIO determining if USB function should be enabled */
if (priv-dparam.enable_gpio) {
gpio_request_one(priv-dparam.enable_gpio, GPIOF_IN, NULL);
ret = !gpio_get_value(priv-dparam.enable_gpio);
gpio_free(priv-dparam.enable_gpio);
if (ret) {


   'ret' is non-zero here if gpio_get_value() returned 0, so 0 means host 
and 1 means gadget, i.e. GPIO is active-high.



dev_warn(pdev-dev,
 USB function not selected (GPIO %d)\n,
 priv-dparam.enable_gpio);
ret = -ENOTSUPP;
goto probe_end_mod_exit;
}
}



Best regards,
Yoshihiro Shimoda


WBR, 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 2/2] arm: shmobile: lager: enable HS-USB

2014-10-06 Thread Sergei Shtylyov

Hello.

On 10/02/2014 12:04 PM, Yoshihiro Shimoda wrote:


Signed-off-by: Yoshihiro Shimoda yoshihiro.shimoda...@renesas.com
---
  arch/arm/boot/dts/r8a7790-lager.dts |5 +
  1 file changed, 5 insertions(+)



diff --git a/arch/arm/boot/dts/r8a7790-lager.dts 
b/arch/arm/boot/dts/r8a7790-lager.dts
index 1698591..4badd0a 100644
--- a/arch/arm/boot/dts/r8a7790-lager.dts
+++ b/arch/arm/boot/dts/r8a7790-lager.dts
@@ -445,3 +445,8 @@
};
};
  };
+
+hsusb {
+   status = okay;
+   renesas,enable-gpio = gpio5 18 GPIO_ACTIVE_LOW;
+};


   Darn, this also misses the PinMux props... I was going to post v2 of these 
patches but now would need to add that and retest... :-/


WBR, 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 2/2] arm: shmobile: lager: enable HS-USB

2014-10-06 Thread Yoshihiro Shimoda
Hello.

(2014/10/07 6:38), Sergei Shtylyov wrote:
 Hello.
 
 On 10/02/2014 12:04 PM, Yoshihiro Shimoda wrote:
 
 Signed-off-by: Yoshihiro Shimoda yoshihiro.shimoda...@renesas.com
 ---
   arch/arm/boot/dts/r8a7790-lager.dts |5 +
   1 file changed, 5 insertions(+)
 
 diff --git a/arch/arm/boot/dts/r8a7790-lager.dts 
 b/arch/arm/boot/dts/r8a7790-lager.dts
 index 1698591..4badd0a 100644
 --- a/arch/arm/boot/dts/r8a7790-lager.dts
 +++ b/arch/arm/boot/dts/r8a7790-lager.dts
 @@ -445,3 +445,8 @@
  };
  };
   };
 +
 +hsusb {
 +status = okay;
 +renesas,enable-gpio = gpio5 18 GPIO_ACTIVE_LOW;
 +};
 
 Darn, this also misses the PinMux props... I was going to post v2 of 
 these 
 patches but now would need to add that and retest... :-/

I'm sorry for the my delayed work... I will submit v2 patches today.

By the way, I cannot understand that this also misses the PinMux props.
According to the original legacy code in board-lager.c, it uses gpio5 18:
https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=36be7686caa05334ca8d52df157b373a41d8d9f1

Best regards,
Yoshihiro Shimoda

 WBR, 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 2/2] arm: shmobile: lager: enable HS-USB

2014-10-06 Thread Yoshihiro Shimoda
Hello.

(2014/10/06 9:59), Yoshihiro Shimoda wrote:
 Hello.
 
 (2014/10/04 4:50), Sergei Shtylyov wrote:
 On 10/02/2014 12:04 PM, Yoshihiro Shimoda wrote:

 Signed-off-by: Yoshihiro Shimoda yoshihiro.shimoda...@renesas.com
 ---
   arch/arm/boot/dts/r8a7790-lager.dts |5 +
   1 file changed, 5 insertions(+)

 diff --git a/arch/arm/boot/dts/r8a7790-lager.dts 
 b/arch/arm/boot/dts/r8a7790-lager.dts
 index 1698591..4badd0a 100644
 --- a/arch/arm/boot/dts/r8a7790-lager.dts
 +++ b/arch/arm/boot/dts/r8a7790-lager.dts
 @@ -445,3 +445,8 @@
 };
 };
   };
 +
 +hsusb {
 +   status = okay;
 +   renesas,enable-gpio = gpio5 18 GPIO_ACTIVE_LOW;

 It's certainly active-high.
 
 Since the current code has the following, we have to set the active_low...
 However, the code is unreadable, I think. So, I will modify the code.
 
   /* check GPIO determining if USB function should be enabled */
   if (priv-dparam.enable_gpio) {
   gpio_request_one(priv-dparam.enable_gpio, GPIOF_IN, NULL);
   ret = !gpio_get_value(priv-dparam.enable_gpio);
   gpio_free(priv-dparam.enable_gpio);
   if (ret) {
   dev_warn(pdev-dev,
USB function not selected (GPIO %d)\n,
priv-dparam.enable_gpio);
   ret = -ENOTSUPP;
   goto probe_end_mod_exit;
   }
   }

I am confusing about the gpio_get_value()...
In case of ARM, gpio_get_value() will call gpiod_get_raw_value() finally.

gpio_get_value() in arch/arm/include/asm/gpio.h
 -- __gpio_get_value() in include/asm-generic/gpio.h
  -- gpiod_get_raw_value() in drivers/gpio/gpiolib.c

The gpiod_get_raw_value() doesn't care of the GPIO_ACTIVE_{HIGH,LOW}.
So, should I add gpiod_is_active_low() or someting in the renesas_usbhs driver?
Or, Do I misunderstand something?

Best regards,
Yoshihiro Shimoda

 Best regards,
 Yoshihiro Shimoda
 
 WBR, 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 2/2] arm: shmobile: lager: enable HS-USB

2014-10-05 Thread Yoshihiro Shimoda
Hello.

(2014/10/04 4:50), Sergei Shtylyov wrote:
 On 10/02/2014 12:04 PM, Yoshihiro Shimoda wrote:
 
 Signed-off-by: Yoshihiro Shimoda yoshihiro.shimoda...@renesas.com
 ---
   arch/arm/boot/dts/r8a7790-lager.dts |5 +
   1 file changed, 5 insertions(+)
 
 diff --git a/arch/arm/boot/dts/r8a7790-lager.dts 
 b/arch/arm/boot/dts/r8a7790-lager.dts
 index 1698591..4badd0a 100644
 --- a/arch/arm/boot/dts/r8a7790-lager.dts
 +++ b/arch/arm/boot/dts/r8a7790-lager.dts
 @@ -445,3 +445,8 @@
  };
  };
   };
 +
 +hsusb {
 +status = okay;
 +renesas,enable-gpio = gpio5 18 GPIO_ACTIVE_LOW;
 
 It's certainly active-high.

Since the current code has the following, we have to set the active_low...
However, the code is unreadable, I think. So, I will modify the code.

/* check GPIO determining if USB function should be enabled */
if (priv-dparam.enable_gpio) {
gpio_request_one(priv-dparam.enable_gpio, GPIOF_IN, NULL);
ret = !gpio_get_value(priv-dparam.enable_gpio);
gpio_free(priv-dparam.enable_gpio);
if (ret) {
dev_warn(pdev-dev,
 USB function not selected (GPIO %d)\n,
 priv-dparam.enable_gpio);
ret = -ENOTSUPP;
goto probe_end_mod_exit;
}
}

Best regards,
Yoshihiro Shimoda

 WBR, 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 2/2] arm: shmobile: lager: enable HS-USB

2014-10-05 Thread Simon Horman
On Mon, Oct 06, 2014 at 09:59:48AM +0900, Yoshihiro Shimoda wrote:
 Hello.
 
 (2014/10/04 4:50), Sergei Shtylyov wrote:
  On 10/02/2014 12:04 PM, Yoshihiro Shimoda wrote:
  
  Signed-off-by: Yoshihiro Shimoda yoshihiro.shimoda...@renesas.com
  ---
arch/arm/boot/dts/r8a7790-lager.dts |5 +
1 file changed, 5 insertions(+)
  
  diff --git a/arch/arm/boot/dts/r8a7790-lager.dts 
  b/arch/arm/boot/dts/r8a7790-lager.dts
  index 1698591..4badd0a 100644
  --- a/arch/arm/boot/dts/r8a7790-lager.dts
  +++ b/arch/arm/boot/dts/r8a7790-lager.dts
  @@ -445,3 +445,8 @@
 };
 };
};
  +
  +hsusb {
  +  status = okay;
  +  renesas,enable-gpio = gpio5 18 GPIO_ACTIVE_LOW;
  
  It's certainly active-high.
 
 Since the current code has the following, we have to set the active_low...
 However, the code is unreadable, I think. So, I will modify the code.

It seems to me that would be best.

As far as possible the bindings and their use should
describe the hardware rather than the software.

   /* check GPIO determining if USB function should be enabled */
   if (priv-dparam.enable_gpio) {
   gpio_request_one(priv-dparam.enable_gpio, GPIOF_IN, NULL);
   ret = !gpio_get_value(priv-dparam.enable_gpio);
   gpio_free(priv-dparam.enable_gpio);
   if (ret) {
   dev_warn(pdev-dev,
USB function not selected (GPIO %d)\n,
priv-dparam.enable_gpio);
   ret = -ENOTSUPP;
   goto probe_end_mod_exit;
   }
   }
 
 Best regards,
 Yoshihiro Shimoda
 
  WBR, 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 2/2] arm: shmobile: lager: enable HS-USB

2014-10-03 Thread Sergei Shtylyov

On 10/02/2014 12:04 PM, Yoshihiro Shimoda wrote:


Signed-off-by: Yoshihiro Shimoda yoshihiro.shimoda...@renesas.com
---
  arch/arm/boot/dts/r8a7790-lager.dts |5 +
  1 file changed, 5 insertions(+)



diff --git a/arch/arm/boot/dts/r8a7790-lager.dts 
b/arch/arm/boot/dts/r8a7790-lager.dts
index 1698591..4badd0a 100644
--- a/arch/arm/boot/dts/r8a7790-lager.dts
+++ b/arch/arm/boot/dts/r8a7790-lager.dts
@@ -445,3 +445,8 @@
};
};
  };
+
+hsusb {
+   status = okay;
+   renesas,enable-gpio = gpio5 18 GPIO_ACTIVE_LOW;


   It's certainly active-high.

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


[PATCH 2/2] arm: shmobile: lager: enable HS-USB

2014-10-02 Thread Yoshihiro Shimoda
Signed-off-by: Yoshihiro Shimoda yoshihiro.shimoda...@renesas.com
---
 arch/arm/boot/dts/r8a7790-lager.dts |5 +
 1 file changed, 5 insertions(+)

diff --git a/arch/arm/boot/dts/r8a7790-lager.dts 
b/arch/arm/boot/dts/r8a7790-lager.dts
index 1698591..4badd0a 100644
--- a/arch/arm/boot/dts/r8a7790-lager.dts
+++ b/arch/arm/boot/dts/r8a7790-lager.dts
@@ -445,3 +445,8 @@
};
};
 };
+
+hsusb {
+   status = okay;
+   renesas,enable-gpio = gpio5 18 GPIO_ACTIVE_LOW;
+};
-- 
1.7.9.5

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