Hi Simon-san,
From: Simon Horman, Sent: Monday, April 9, 2018 8:58 PM
On Mon, Apr 02, 2018 at 09:21:34PM +0900, Yoshihiro Shimoda wrote:
This patch fixes an issue that this driver cannot call phy_init() if a gadget driver is alreadly loaded because usb_add_gadget_udc() might call renesas_usb3_start() via .udc_start.
Fixes: 279d4bc64060 ("usb: gadget: udc: renesas_usb3: add support for generic phy") Cc: stable@vger.kernel.org # v4.15+ Signed-off-by: Yoshihiro Shimoda yoshihiro.shimoda.uh@renesas.com
drivers/usb/gadget/udc/renesas_usb3.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-)
Reviewed-by: Simon Horman horms+renesas@verge.net.au
Please see some suggestions for follow-up lower-priority patches below.
Thank you for the review and suggestions!
diff --git a/drivers/usb/gadget/udc/renesas_usb3.c b/drivers/usb/gadget/udc/renesas_usb3.c index 738b734..671bac3 100644 --- a/drivers/usb/gadget/udc/renesas_usb3.c +++ b/drivers/usb/gadget/udc/renesas_usb3.c @@ -2632,6 +2632,14 @@ static int renesas_usb3_probe(struct platform_device *pdev) if (ret < 0) goto err_alloc_prd;
- /*
* This is an optional. So, if this driver cannot get a phy,* this driver will not handle a phy anymore.*/nit: s/an optional/optional/
Oops. I will fix and submit v2 patches.
- usb3->phy = devm_phy_get(&pdev->dev, "usb");
- if (IS_ERR(usb3->phy))
usb3->phy = NULL;I think this will unintentionally ignore errors other than the non-existence of the device, f.e. a memory allocation failure in devm_phy_get().
So perhaps something like this, as per phy_optional_get(), would be better?
usb3->phy = devm_phy_get(&pdev->dev, "usb"); if (IS_ERR(usb3->phy) && (PTR_ERR(usb3->phy) == -ENODEV)) usb3->phy = NULL;
Thank you for the suggestion. I agree with you. I think I should use devm_phy_optional_get() instead of dev_phy_get(), as you mentioned :) So, I will fix the code by using devm_phy_optional_get() first, and then fix this.
Best regards, Yoshihiro Shimoda
- pm_runtime_enable(&pdev->dev); ret = usb_add_gadget_udc(&pdev->dev, &usb3->gadget); if (ret < 0)
@@ -2641,14 +2649,6 @@ static int renesas_usb3_probe(struct platform_device *pdev) if (ret < 0) goto err_dev_create;
/*
* This is an optional. So, if this driver cannot get a phy,* this driver will not handle a phy anymore.*/usb3->phy = devm_phy_get(&pdev->dev, "usb");
if (IS_ERR(usb3->phy))
usb3->phy = NULL;usb3->workaround_for_vbus = priv->workaround_for_vbus;
renesas_usb3_debugfs_init(usb3, &pdev->dev);
-- 1.9.1