[PATCH] can: flexcan: add a regulator for transceiver

On Tue, Jul 03, 2012 at 03:46:47PM +0100, Mark Brown wrote:

On Tue, Jul 03, 2012 at 10:18:31PM +0800, Shawn Guo wrote:

quoted

On Tue, Jul 03, 2012 at 02:07:47PM +0100, Mark Brown wrote:

quoted

quoted

No, the regulator API will stub itself out if not enabled, and if the
supply is fixed then a fixed voltage regulator will do the job.  We
shouldn't be open coding this stuff in individual users.

quoted

Ah, yes.  But when you say "a fixed voltage regulator", you actually
meant dummy regulator, right?

No, I really do mean a fixed voltage regulator.  Dummy regulators should
essentially never be used in production.

In that case, my patch may need to stay unchanged.  Some systems have
a supply controlled by gpio and we need to have it be a Linux regulator
to switch it, while other systems have the supply non-switchable which
can be reasonably invisible to software.  That said, the regulator is
optional and the failure of devm_regulator_get should not make the
driver probe fail.

I guess your suggestion here is to make the regulator mandatory for
the driver, which means those non-switchable supplies have to be
also defined as Linux regulators for every single system.  I'm not
sure we want to do that just for saving a regulator pointer check.

quoted

 static void flexcan_transceiver_switch(const struct flexcan_priv *priv, int on)
 {
+       if (on)
+               regulator_enable(priv->reg_xcvr);
+       else
+               regulator_disable(priv->reg_xcvr);
+

So long as the stack guarantees that you won't get unbalanced calls to
this and...

quoted

+       priv->reg_xcvr = devm_regulator_get(&pdev->dev, "xcvr");

...add error checking here the code should be fine, yes.

-- 
Regards,
Shawn