Re: [PATCH v3 1/6] usb: host: usb-st-common: Add common code required by ohci-st and ehci-st

2014-08-07 Thread Peter Griffin 
Hi Arnd,

Thanks for reviewing, see my comments below: -

  +   if (priv-rst) {
  +   ret = 
  (priv-rst);
  +   if (ret)
  +   goto err_assert_power;
  +   }
 
 I wouldn't make these optional, just call the functions
 unconditionally and fail the probe function if they are
 not available.
 
 I'm not sure if it's worth keeping these functions in a
 common file. You are adding complexity this way and I don't
 think you are even saving a significant number of code lines
 compared to just having two copies of them.

I've unabstracted these common functions back into ehci-st.c and 
ohci-st,c in V4.

regards,

Peter.

--
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 v3 1/6] usb: host: usb-st-common: Add common code required by ohci-st and ehci-st

2014-08-07 Thread Peter Griffin 
Hi Arnd,

   (priv-rst);
   + if (ret)
   + goto err_assert_power;
   + }
  
  I wouldn't make these optional, just call the functions
  unconditionally and fail the probe function if they are
  not available.

Also I've also done as you suggest and made
these non optional in V4.

regards,

Peter.
--
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 v3 1/6] usb: host: usb-st-common: Add common code required by ohci-st and ehci-st

2014-08-06 Thread Arnd Bergmann 
On Wednesday 06 August 2014, Peter Griffin wrote:
 +int st_usb_platform_power_on(struct st_platform_priv *priv)
 +{
 + int clk, ret;
 +
 + if (priv-pwr) {
 + ret = reset_control_deassert(priv-pwr);
 + if (ret)
 + return ret;
 + }
 +
 + if (priv-rst) {
 + ret = 
 (priv-rst);
 + if (ret)
 + goto err_assert_power;
 + }

I wouldn't make these optional, just call the functions
unconditionally and fail the probe function if they are
not available.

I'm not sure if it's worth keeping these functions in a
common file. You are adding complexity this way and I don't
think you are even saving a significant number of code lines
compared to just having two copies of them.

 +EXPORT_SYMBOL(st_usb_platform_power_on);

If you want to keep them, it would be best to make

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