On Mon, May 27, 2013 at 09:51:28AM -0300, Fabio Estevam wrote:
> There is no need to keep a 'reg_vbus' indirection, so get rid of it.
>
> The motivation for doing this change is that in the case of error, the current
> code only sets the local reg_vbus to NULL instead of updating the private
> structure 'data->reg_vbus'.
>
> Updating only the local reg_vbus is wrong, since we currently check for
> data->reg_vbus in the ci13xxx_imx_remove() function.
>
> In order to avoid such issue, just use 'data->reg_vbus' directly.
>
Do you actually see the issue in real? While I'm fine with the change,
I do not think the current code will cause the issue you are describing
here, since the devm_kzalloc() of data ensures that data->reg_vbus is NULL
initially.
Shawn
> Signed-off-by: Fabio Estevam <fabio.estevam@xxxxxxxxxxxxx>
> ---
> drivers/usb/chipidea/ci13xxx_imx.c | 14 ++++++--------
> 1 file changed, 6 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/usb/chipidea/ci13xxx_imx.c b/drivers/usb/chipidea/ci13xxx_imx.c
> index a9afd06..1661cac 100644
> --- a/drivers/usb/chipidea/ci13xxx_imx.c
> +++ b/drivers/usb/chipidea/ci13xxx_imx.c
> @@ -102,7 +102,6 @@ static int ci13xxx_imx_probe(struct platform_device *pdev)
> struct platform_device *phy_pdev;
> struct device_node *phy_np;
> struct resource *res;
> - struct regulator *reg_vbus;
> struct pinctrl *pinctrl;
> int ret;
>
> @@ -157,18 +156,17 @@ static int ci13xxx_imx_probe(struct platform_device *pdev)
> }
>
> /* we only support host now, so enable vbus here */
> - reg_vbus = devm_regulator_get(&pdev->dev, "vbus");
> - if (!IS_ERR(reg_vbus)) {
> - ret = regulator_enable(reg_vbus);
> + data->reg_vbus = devm_regulator_get(&pdev->dev, "vbus");
> + if (!IS_ERR(data->reg_vbus)) {
> + ret = regulator_enable(data->reg_vbus);
> if (ret) {
> dev_err(&pdev->dev,
> "Failed to enable vbus regulator, err=%d\n",
> ret);
> goto put_np;
> }
> - data->reg_vbus = reg_vbus;
> } else {
> - reg_vbus = NULL;
> + data->reg_vbus = NULL;
> }
>
> ci13xxx_imx_platdata.phy = data->phy;
> @@ -217,8 +215,8 @@ static int ci13xxx_imx_probe(struct platform_device *pdev)
> disable_device:
> ci13xxx_remove_device(data->ci_pdev);
> err:
> - if (reg_vbus)
> - regulator_disable(reg_vbus);
> + if (data->reg_vbus)
> + regulator_disable(data->reg_vbus);
> put_np:
> if (phy_np)
> of_node_put(phy_np);
> --
> 1.8.1.2
>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
