Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <c7d4c1d4-f337-a425-bf0d-f7b0f03dab7c@quicinc.com>
Date:   Fri, 12 May 2023 10:18:14 -0700
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:91.0) Gecko/20100101
 Thunderbird/91.9.1
Subject: Re: [PATCH 2/2] usb: dwc3: Modify runtime pm ops to handle bus
 suspend
Content-Language: en-US
To:     Dan Carpenter <dan.carpenter@linaro.org>,
        <oe-kbuild@lists.linux.dev>, <gregkh@linuxfoundation.org>,
        <Thinh.Nguyen@synopsys.com>
CC:     <lkp@intel.com>, <oe-kbuild-all@lists.linux.dev>,
        <linux-kernel@vger.kernel.org>, <linux-usb@vger.kernel.org>,
        <quic_wcheng@quicinc.com>, <quic_jackp@quicinc.com>
References: <3e035312-3c06-44e1-95ef-0a4d36456a7d@kili.mountain>
From:   Elson Serrao <quic_eserrao@quicinc.com>
In-Reply-To: <3e035312-3c06-44e1-95ef-0a4d36456a7d@kili.mountain>
Content-Type: text/plain; charset="UTF-8"; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk


On 5/11/2023 9:18 PM, Dan Carpenter wrote:
> Hi Elson,
> 
> kernel test robot noticed the following build warnings:
> 
> https://git-scm.com/docs/git-format-patch#_base_tree_information]
> 
> url:    https://github.com/intel-lab-lkp/linux/commits/Elson-Roy-Serrao/usb-function-u_ether-Handle-rx-requests-during-suspend-resume/20230512-015036
> base:   https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git usb-testing
> patch link:    https://lore.kernel.org/r/1683827311-1462-3-git-send-email-quic_eserrao%40quicinc.com
> patch subject: [PATCH 2/2] usb: dwc3: Modify runtime pm ops to handle bus suspend
> config: x86_64-randconfig-m001 (https://download.01.org/0day-ci/archive/20230512/202305120709.tCFYCtsd-lkp@intel.com/config)
> compiler: gcc-11 (Debian 11.3.0-12) 11.3.0
> 
> If you fix the issue, kindly add following tag where applicable
> | Reported-by: kernel test robot <lkp@intel.com>
> | Reported-by: Dan Carpenter <error27@gmail.com>
> | Link: https://lore.kernel.org/r/202305120709.tCFYCtsd-lkp@intel.com/
> 
> New smatch warnings:
> drivers/usb/dwc3/gadget.c:2409 dwc3_gadget_wakeup() warn: pm_runtime_get_sync() also returns 1 on success
> drivers/usb/dwc3/gadget.c:2437 dwc3_gadget_func_wakeup() warn: pm_runtime_get_sync() also returns 1 on success
> 
> Old smatch warnings:
> drivers/usb/dwc3/gadget.c:1648 __dwc3_gadget_kick_transfer() warn: missing error code? 'ret'
> drivers/usb/dwc3/gadget.c:2744 dwc3_gadget_pullup() warn: pm_runtime_get_sync() also returns 1 on success
> 
> vim +2409 drivers/usb/dwc3/gadget.c
> 
> 218ef7b647e336 Felipe Balbi     2016-04-04  2392  static int dwc3_gadget_wakeup(struct usb_gadget *g)
> 218ef7b647e336 Felipe Balbi     2016-04-04  2393  {
> 218ef7b647e336 Felipe Balbi     2016-04-04  2394  	struct dwc3		*dwc = gadget_to_dwc(g);
> 218ef7b647e336 Felipe Balbi     2016-04-04  2395  	unsigned long		flags;
> 218ef7b647e336 Felipe Balbi     2016-04-04  2396  	int			ret;
> 218ef7b647e336 Felipe Balbi     2016-04-04  2397
> 047161686b813a Elson Roy Serrao 2023-03-24  2398  	if (!dwc->wakeup_configured) {
> 047161686b813a Elson Roy Serrao 2023-03-24  2399  		dev_err(dwc->dev, "remote wakeup not configured\n");
> 047161686b813a Elson Roy Serrao 2023-03-24  2400  		return -EINVAL;
> 047161686b813a Elson Roy Serrao 2023-03-24  2401  	}
> 047161686b813a Elson Roy Serrao 2023-03-24  2402
> 047161686b813a Elson Roy Serrao 2023-03-24  2403  	if (!dwc->gadget->wakeup_armed) {
> 047161686b813a Elson Roy Serrao 2023-03-24  2404  		dev_err(dwc->dev, "not armed for remote wakeup\n");
> 047161686b813a Elson Roy Serrao 2023-03-24  2405  		return -EINVAL;
> 047161686b813a Elson Roy Serrao 2023-03-24  2406  	}
> 047161686b813a Elson Roy Serrao 2023-03-24  2407
> 0660b8a88d4d6a Elson Roy Serrao 2023-05-11  2408  	ret = pm_runtime_get_sync(dwc->dev);
> 0660b8a88d4d6a Elson Roy Serrao 2023-05-11 @2409  	if (ret) {
> 
> The checker is correct.  These days it's better to use the
> pm_runtime_resume_and_get() function instead of pm_runtime_get_sync().
> 

Sure.Thanks!

> 0660b8a88d4d6a Elson Roy Serrao 2023-05-11  2410  		pm_runtime_put(dwc->dev);
> 0660b8a88d4d6a Elson Roy Serrao 2023-05-11  2411  		return ret;
> 0660b8a88d4d6a Elson Roy Serrao 2023-05-11  2412  	}
> 0660b8a88d4d6a Elson Roy Serrao 2023-05-11  2413
> 0660b8a88d4d6a Elson Roy Serrao 2023-05-11  2414  	spin_lock_irqsave(&dwc->lock, flags);
> 0660b8a88d4d6a Elson Roy Serrao 2023-05-11  2415  	ret = __dwc3_gadget_wakeup(dwc, true);
> 72246da40f3719 Felipe Balbi     2011-08-19  2416  	spin_unlock_irqrestore(&dwc->lock, flags);
> 0660b8a88d4d6a Elson Roy Serrao 2023-05-11  2417  	pm_runtime_put_noidle(dwc->dev);
> 72246da40f3719 Felipe Balbi     2011-08-19  2418
> 72246da40f3719 Felipe Balbi     2011-08-19  2419  	return ret;
> 72246da40f3719 Felipe Balbi     2011-08-19  2420  }
> 72246da40f3719 Felipe Balbi     2011-08-19  2421
> 92c08a84b53e5d Elson Roy Serrao 2023-03-24  2422  static void dwc3_resume_gadget(struct dwc3 *dwc);
> 92c08a84b53e5d Elson Roy Serrao 2023-03-24  2423
> 92c08a84b53e5d Elson Roy Serrao 2023-03-24  2424  static int dwc3_gadget_func_wakeup(struct usb_gadget *g, int intf_id)
> 92c08a84b53e5d Elson Roy Serrao 2023-03-24  2425  {
> 92c08a84b53e5d Elson Roy Serrao 2023-03-24  2426  	struct  dwc3		*dwc = gadget_to_dwc(g);
> 92c08a84b53e5d Elson Roy Serrao 2023-03-24  2427  	unsigned long		flags;
> 92c08a84b53e5d Elson Roy Serrao 2023-03-24  2428  	int			ret;
> 92c08a84b53e5d Elson Roy Serrao 2023-03-24  2429  	int			link_state;
> 92c08a84b53e5d Elson Roy Serrao 2023-03-24  2430
> 92c08a84b53e5d Elson Roy Serrao 2023-03-24  2431  	if (!dwc->wakeup_configured) {
> 92c08a84b53e5d Elson Roy Serrao 2023-03-24  2432  		dev_err(dwc->dev, "remote wakeup not configured\n");
> 92c08a84b53e5d Elson Roy Serrao 2023-03-24  2433  		return -EINVAL;
> 92c08a84b53e5d Elson Roy Serrao 2023-03-24  2434  	}
> 92c08a84b53e5d Elson Roy Serrao 2023-03-24  2435
> 0660b8a88d4d6a Elson Roy Serrao 2023-05-11  2436  	ret = pm_runtime_get_sync(dwc->dev);
> 0660b8a88d4d6a Elson Roy Serrao 2023-05-11 @2437  	if (ret) {
> 
> Same.
> 
> 0660b8a88d4d6a Elson Roy Serrao 2023-05-11  2438  		pm_runtime_put(dwc->dev);
> 0660b8a88d4d6a Elson Roy Serrao 2023-05-11  2439  		return ret;
> 0660b8a88d4d6a Elson Roy Serrao 2023-05-11  2440  	}
> 0660b8a88d4d6a Elson Roy Serrao 2023-05-11  2441
> 92c08a84b53e5d Elson Roy Serrao 2023-03-24  2442  	spin_lock_irqsave(&dwc->lock, flags);
> 92c08a84b53e5d Elson Roy Serrao 2023-03-24  2443  	/*
> 92c08a84b53e5d Elson Roy Serrao 2023-03-24  2444  	 * If the link is in U3, signal for remote wakeup and wait for the
> 92c08a84b53e5d Elson Roy Serrao 2023-03-24  2445  	 * link to transition to U0 before sending device notification.
> 92c08a84b53e5d Elson Roy Serrao 2023-03-24  2446  	 */
> 92c08a84b53e5d Elson Roy Serrao 2023-03-24  2447  	link_state = dwc3_gadget_get_link_state(dwc);
> 92c08a84b53e5d Elson Roy Serrao 2023-03-24  2448  	if (link_state == DWC3_LINK_STATE_U3) {
> 92c08a84b53e5d Elson Roy Serrao 2023-03-24  2449  		ret = __dwc3_gadget_wakeup(dwc, false);
> 92c08a84b53e5d Elson Roy Serrao 2023-03-24  2450  		if (ret) {
> 92c08a84b53e5d Elson Roy Serrao 2023-03-24  2451  			spin_unlock_irqrestore(&dwc->lock, flags);
> 0660b8a88d4d6a Elson Roy Serrao 2023-05-11  2452  			pm_runtime_put_noidle(dwc->dev);
> 92c08a84b53e5d Elson Roy Serrao 2023-03-24  2453  			return -EINVAL;
> 92c08a84b53e5d Elson Roy Serrao 2023-03-24  2454  		}
> 92c08a84b53e5d Elson Roy Serrao 2023-03-24  2455  		dwc3_resume_gadget(dwc);
> 92c08a84b53e5d Elson Roy Serrao 2023-03-24  2456  		dwc->link_state = DWC3_LINK_STATE_U0;
> 92c08a84b53e5d Elson Roy Serrao 2023-03-24  2457  	}
> 92c08a84b53e5d Elson Roy Serrao 2023-03-24  2458
> 92c08a84b53e5d Elson Roy Serrao 2023-03-24  2459  	ret = dwc3_send_gadget_generic_command(dwc, DWC3_DGCMD_DEV_NOTIFICATION,
> 92c08a84b53e5d Elson Roy Serrao 2023-03-24  2460  					       DWC3_DGCMDPAR_DN_FUNC_WAKE |
> 92c08a84b53e5d Elson Roy Serrao 2023-03-24  2461  					       DWC3_DGCMDPAR_INTF_SEL(intf_id));
> 92c08a84b53e5d Elson Roy Serrao 2023-03-24  2462  	if (ret)
> 92c08a84b53e5d Elson Roy Serrao 2023-03-24  2463  		dev_err(dwc->dev, "function remote wakeup failed, ret:%d\n", ret);
> 92c08a84b53e5d Elson Roy Serrao 2023-03-24  2464
> 92c08a84b53e5d Elson Roy Serrao 2023-03-24  2465  	spin_unlock_irqrestore(&dwc->lock, flags);
> 0660b8a88d4d6a Elson Roy Serrao 2023-05-11  2466  	pm_runtime_put_noidle(dwc->dev);
> 92c08a84b53e5d Elson Roy Serrao 2023-03-24  2467
> 92c08a84b53e5d Elson Roy Serrao 2023-03-24  2468  	return ret;
> 92c08a84b53e5d Elson Roy Serrao 2023-03-24  2469  }
>