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: <b75076e5-1137-bad1-d9ae-bb9b6945a5c3@windriver.com>
Date:   Thu, 15 Jun 2023 21:14:29 +1100
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.10.0
Subject: Re: [PATCH 1/1] usb: cdns3: Put the cdns set active part outside the
 spin lock
Content-Language: en-US
To:     Greg KH <gregkh@linuxfoundation.org>
Cc:     peter.chen@kernel.org, pawell@cadence.com, rogerq@kernel.org,
        a-govindraju@ti.com, linux-usb@vger.kernel.org,
        linux-kernel@vger.kernel.org
References: <20230504131602.3273835-1-xiaolei.wang@windriver.com>
 <2023050503-handful-shore-ae11@gregkh>
 <3a9c2750-1c80-506a-69ec-a92250dfeaa8@windriver.com>
 <2023061507-mummy-salvation-c45a@gregkh>
From:   wang xiaolei <xiaolei.wang@windriver.com>
In-Reply-To: <2023061507-mummy-salvation-c45a@gregkh>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?Z3hRM2xnK3ZoSjRiU3dnbTk5c2RsS0dCQzdOZkF5eXJzbXhCcjlMZk52dE5U?=
 =?utf-8?B?Y2NJVGcxc3JjWFNNdGpoc3VWMmNxV0tLVG9ac3Q1VUpXSDAyeCtkSDNsaFB2?=
 =?utf-8?B?dFFsWGh1OUZuaGtxWG95cTFhbldyNlJKUUxwTzBrcjZlQmZLZkI3OUxQM3Rk?=
 =?utf-8?B?QW1adUZiWC91ZVc2MC9VVFVIY1FSS2NrdEFwS3RTZWlBV2cxWVByd2grWG1a?=
 =?utf-8?B?UGhDVE5YTStMekE2L3RVN2RSOGwrRWNGY2t4OUx0UVZ2VmtSeXh1S3BZcnNr?=
 =?utf-8?B?enI5KzRsWnZReTE3ZHhuTDl2MkJOek83UXNiSXFRSzNnWnQ0WndnYWIxbm5v?=
 =?utf-8?B?c2JpNm5uSTNMSXpEREw5WHR6YkJWbjRraGU5NEx1QkxUN0c2UVpxYzltOUJZ?=
 =?utf-8?B?VndqVCtrZHVqUS9PQkx3ek1ibjd2bHFUVGNlR1NrRm1JNURHK0Q4Q3dzSkNV?=
 =?utf-8?B?ekpBaVA3V1dtL1VLeDNFQkJPVHJXTUZSaVg3ZTVFL1lmM3EyQ2NxUjQvVVJQ?=
 =?utf-8?B?VEtWb2lCajl4SkRiV2pPelBMdFlZT2tGSk9vdHN3aVpZeERiKzMxdEd3dHBT?=
 =?utf-8?B?MFlXVXlLVlhGYVljL1hERHBBdGFWWTAyY3pCT1ZZMVUydFZJbEREWEhlMkhL?=
 =?utf-8?B?b2pJYk9rbDFkR3pKbEJKZnlSYnBVeUVVUFdOdlJHU3JiNlJKOEFZWHZNbGVl?=
 =?utf-8?B?NFIvZnlWWG9LYXBlTVcyeVdjT0I0Y0lqU0VCK2hydmNVZTNZWGFuU3NYVjFp?=
 =?utf-8?B?TlpIYm1peURZVDZ4R0kxaE5tYU4xMWg5SFJZRStYS1ZQaXV2cFVuVDEvV2xs?=
 =?utf-8?B?NG1ndlBBSVF6YWFsb3BFc1VRc3ZoQnZYaHF4dUVFdm90TlNqY29wNjAwNXRR?=
 =?utf-8?B?MW9JTFhrM3FMU1dqUEtyZzQwUUlvYzk3YXA5Z01aeGJYT3RFMGJDeUdoTUw1?=
 =?utf-8?B?dG9jakFQWXIxQ0Z1eWNTcitHZVlzMEVXaGIvUkVwM3NPQXdjS3lqcU1oRlhQ?=
 =?utf-8?B?N2pZRGxoZ1FjdW84eTZTZ3ZhYVlXTUZEZndmbkNKWmZMeTg1Z2dNeFR6Z0l6?=
 =?utf-8?B?WDRRMHdOUUNqeElwZEhJSzJRdmZqOEh3ZGE2b0JnQ05yN0xEUkU4SUtqSWxi?=
 =?utf-8?B?WlNXM3RyQlVqL0Y3cVNOUlFBekc2Y2NYQU1mUDF4Vjg0c2JZNlVLSUQvL1gv?=
 =?utf-8?B?dmgvS2piRUk2T1luVzBmT2RaeUYyYnVHemppSm8xNGJ6N0FwVVJSaElCZ202?=
 =?utf-8?B?Wk8yL0VRdW5tSU1DZ1kwTURLUEpxOEJiYUxFNmxwQ0lQV1Jnd3VqSlNqK3Bm?=
 =?utf-8?B?a0RkV3NXSUV3TG5hd2E1S3oxZFF3bFZublFTMmJISjlSNk03dDUwcjFnQlpN?=
 =?utf-8?B?U0U3T3R4WElaTDkwVUlKWEh6UEVBV1FxdWM2cVdud0NqVTQ2NGtURmdRbDYz?=
 =?utf-8?B?WUExWVdUckY3eFFhR0pNV251Z0h3ZHdUenVwK1d5Qk9DZ0QwUVdSU1VtZWtx?=
 =?utf-8?B?aVI1V3hSaVBFTWtDbEJla1BMUC9ITEhLeVdKblRoZTR6OWdWTTN3WUY2czh3?=
 =?utf-8?B?UXZWTG5tUFdSYmtmQ3BwOWttWFVZT2RrMWdPcittcXB1TjQ2aXcwY0hyN2t1?=
 =?utf-8?B?ckVsNXhBRGJKYWhhVW9tQ0JhK3QvSmlqcCt1SC9BYUJtN2J6YVdyNWRsWnZR?=
 =?utf-8?B?Y1NMdHFEUlVSQ08zejZpRkVOaWxLSDhXc2xYazFLZk96bm9rYXpTanFsT3A1?=
 =?utf-8?B?YWhPUkFiNGlCZHlVd2hwelpUMnNsUURXTk1Na2c2amlzWURkWTlBL2RCL0hh?=
 =?utf-8?B?enZud0Rnd3ptNEp4RE5VUjJ6OEQyY0xmTi9MbW9CUGdwdCtNeEROWnZ1Z0s3?=
 =?utf-8?B?N0dUQTdxS1NxUVFoa1BOWVlLUDJTODl2TFR4T2ZQejU1MitGL1A0bXF5QUM3?=
 =?utf-8?B?RUdQVVRJd0c5Y1duTHZTY2pkaHZYMXVPY0o2VXlzaHZibnkzLzhGeUtXZEZq?=
 =?utf-8?B?RWowbXRhZnhucU1nendhK1lxUi92am5Qb1hXWFFvWGlnQXpLcy8yOXVPWmlj?=
 =?utf-8?B?NHV4SDJKMzlKVW9qNTZQVFNDdy9DY3hBRXBoL1lxKzdVOGR2emtQamNycjhF?=
 =?utf-8?B?OE1MUlhyREU2TkNob3llM3JZOURZZWtZcGNGZUtmaWdpeWpSNU5CUlJoZDVT?=
 =?utf-8?B?bDVqdXFPR21FR2pheHBwYldaOThlUkljeFVOZHZnd2FoOUNkRmZycnRGRnRn?=
 =?utf-8?B?dkYzQm5sNXVTUEo2OEo3QTYxMlJ3PT0=?=
X-OriginatorOrg: windriver.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 6bbf1264-943a-4335-fe1e-08db6d8954e0
X-MS-Exchange-CrossTenant-AuthSource: MW5PR11MB5764.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 15 Jun 2023 10:14:40.6206
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 8ddb2873-a1ad-4a18-ae4e-4644631433be
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: OKRw84hAFf9yJ1nLRULFchJzXhCwNkEXP505SXcJm4fhCNPTtp8QrFxjifzgiOE33wsVTnH94YSLycavSs39GPVZn3df+eyayJcGpegVzro=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM4PR11MB6041
X-Proofpoint-ORIG-GUID: a-6A--q_YA8-qtWc5mrOFl2JK1SlLudU
X-Proofpoint-GUID: a-6A--q_YA8-qtWc5mrOFl2JK1SlLudU
X-Proofpoint-Virus-Version: vendor=baseguard
 engine=ICAP:2.0.254,Aquarius:18.0.957,Hydra:6.0.573,FMLib:17.11.176.26
 definitions=2023-06-15_06,2023-06-14_02,2023-05-22_02
X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 phishscore=0 mlxlogscore=999
 lowpriorityscore=0 suspectscore=0 priorityscore=1501 impostorscore=0
 bulkscore=0 spamscore=0 mlxscore=0 malwarescore=0 clxscore=1015
 adultscore=0 classifier=spam adjust=0 reason=mlx scancount=1
 engine=8.12.0-2305260000 definitions=main-2306150088
X-Spam-Status: No, score=-2.9 required=5.0 tests=BAYES_00,DKIM_SIGNED,
        DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,NICE_REPLY_A,RCVD_IN_DNSWL_LOW,
        RCVD_IN_MSPIKE_H3,RCVD_IN_MSPIKE_WL,SPF_HELO_NONE,SPF_PASS,
        T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org


On 6/15/23 8:49 PM, Greg KH wrote:
> CAUTION: This email comes from a non Wind River email account!
> Do not click links or open attachments unless you recognize the sender and know the content is safe.
>
> On Wed, Jun 14, 2023 at 11:16:05AM +0800, wangxiaolei wrote:
>> On 5/5/23 6:33 PM, Greg KH wrote:
>>> CAUTION: This email comes from a non Wind River email account!
>>> Do not click links or open attachments unless you recognize the sender and know the content is safe.
>>>
>>> On Thu, May 04, 2023 at 09:16:02PM +0800, Xiaolei Wang wrote:
>>>> The device may be scheduled during the resume process,
>>>> so this cannot appear in atomic operations. Since
>>>> pm_runtime_set_active will resume suppliers, put set
>>>> active outside the spin lock, which is only used to
>>>> protect the struct cdns data structure, otherwise the
>>>> kernel will report the following warning:
>>>>
>>>>     BUG: sleeping function called from invalid context at drivers/base/power/runtime.c:1163
>>>>     in_atomic(): 1, irqs_disabled(): 0, non_block: 0, pid: 651, name: sh
>>>>     preempt_count: 1, expected: 0
>>>>     RCU nest depth: 0, expected: 0
>>>>     CPU: 0 PID: 651 Comm: sh Tainted: G        WC         6.1.20 #1
>>>>     Hardware name: Freescale i.MX8QM MEK (DT)
>>>>     Call trace:
>>>>       dump_backtrace.part.0+0xe0/0xf0
>>>>       show_stack+0x18/0x30
>>>>       dump_stack_lvl+0x64/0x80
>>>>       dump_stack+0x1c/0x38
>>>>       __might_resched+0x1fc/0x240
>>>>       __might_sleep+0x68/0xc0
>>>>       __pm_runtime_resume+0x9c/0xe0
>>>>       rpm_get_suppliers+0x68/0x1b0
>>>>       __pm_runtime_set_status+0x298/0x560
>>>>       cdns_resume+0xb0/0x1c0
>>>>       cdns3_controller_resume.isra.0+0x1e0/0x250
>>>>       cdns3_plat_resume+0x28/0x40
>>>>
>>>> Signed-off-by: Xiaolei Wang <xiaolei.wang@windriver.com>
>>>> ---
>>>>    drivers/usb/cdns3/cdns3-plat.c |  3 ++-
>>>>    drivers/usb/cdns3/cdnsp-pci.c  |  3 ++-
>>>>    drivers/usb/cdns3/core.c       | 14 +++++++++++---
>>>>    drivers/usb/cdns3/core.h       |  3 ++-
>>>>    4 files changed, 17 insertions(+), 6 deletions(-)
>>>>
>>>> diff --git a/drivers/usb/cdns3/cdns3-plat.c b/drivers/usb/cdns3/cdns3-plat.c
>>>> index 2bc5d094548b..726b2e4f67e4 100644
>>>> --- a/drivers/usb/cdns3/cdns3-plat.c
>>>> +++ b/drivers/usb/cdns3/cdns3-plat.c
>>>> @@ -256,9 +256,10 @@ static int cdns3_controller_resume(struct device *dev, pm_message_t msg)
>>>>         cdns3_set_platform_suspend(cdns->dev, false, false);
>>>>
>>>>         spin_lock_irqsave(&cdns->lock, flags);
>>>> -     cdns_resume(cdns, !PMSG_IS_AUTO(msg));
>>>> +     cdns_resume(cdns);
>>>>         cdns->in_lpm = false;
>>>>         spin_unlock_irqrestore(&cdns->lock, flags);
>>>> +     cdns_set_active(cdns, !PMSG_IS_AUTO(msg));
>>>>         if (cdns->wakeup_pending) {
>>>>                 cdns->wakeup_pending = false;
>>>>                 enable_irq(cdns->wakeup_irq);
>>>> diff --git a/drivers/usb/cdns3/cdnsp-pci.c b/drivers/usb/cdns3/cdnsp-pci.c
>>>> index 7b151f5af3cc..0725668ffea4 100644
>>>> --- a/drivers/usb/cdns3/cdnsp-pci.c
>>>> +++ b/drivers/usb/cdns3/cdnsp-pci.c
>>>> @@ -208,8 +208,9 @@ static int __maybe_unused cdnsp_pci_resume(struct device *dev)
>>>>         int ret;
>>>>
>>>>         spin_lock_irqsave(&cdns->lock, flags);
>>>> -     ret = cdns_resume(cdns, 1);
>>>> +     ret = cdns_resume(cdns);
>>>>         spin_unlock_irqrestore(&cdns->lock, flags);
>>>> +     cdns_set_active(cdns, 1);
>>>>
>>>>         return ret;
>>>>    }
>>>> diff --git a/drivers/usb/cdns3/core.c b/drivers/usb/cdns3/core.c
>>>> index dbcdf3b24b47..661d5597fb68 100644
>>>> --- a/drivers/usb/cdns3/core.c
>>>> +++ b/drivers/usb/cdns3/core.c
>>>> @@ -522,7 +522,7 @@ int cdns_suspend(struct cdns *cdns)
>>>>    }
>>>>    EXPORT_SYMBOL_GPL(cdns_suspend);
>>>>
>>>> -int cdns_resume(struct cdns *cdns, u8 set_active)
>>>> +int cdns_resume(struct cdns *cdns)
>>>>    {
>>>>         struct device *dev = cdns->dev;
>>>>         enum usb_role real_role;
>>>> @@ -556,15 +556,23 @@ int cdns_resume(struct cdns *cdns, u8 set_active)
>>>>         if (cdns->roles[cdns->role]->resume)
>>>>                 cdns->roles[cdns->role]->resume(cdns, cdns_power_is_lost(cdns));
>>>>
>>>> +     return 0;
>>>> +}
>>>> +EXPORT_SYMBOL_GPL(cdns_resume);
>>>> +
>>>> +void cdns_set_active(struct cdns *cdns, u8 set_active)
>>>> +{
>>>> +     struct device *dev = cdns->dev;
>>>> +
>>>>         if (set_active) {
>>>>                 pm_runtime_disable(dev);
>>>>                 pm_runtime_set_active(dev);
>>>>                 pm_runtime_enable(dev);
>>>>         }
>>>>
>>>> -     return 0;
>>>> +     return;
>>>>    }
>>>> -EXPORT_SYMBOL_GPL(cdns_resume);
>>>> +EXPORT_SYMBOL_GPL(cdns_set_active);
>>>>    #endif /* CONFIG_PM_SLEEP */
>>>>
>>>>    MODULE_AUTHOR("Peter Chen <peter.chen@nxp.com>");
>>>> diff --git a/drivers/usb/cdns3/core.h b/drivers/usb/cdns3/core.h
>>>> index 2d332a788871..0f429042f997 100644
>>>> --- a/drivers/usb/cdns3/core.h
>>>> +++ b/drivers/usb/cdns3/core.h
>>>> @@ -125,8 +125,9 @@ int cdns_init(struct cdns *cdns);
>>>>    int cdns_remove(struct cdns *cdns);
>>>>
>>>>    #ifdef CONFIG_PM_SLEEP
>>>> -int cdns_resume(struct cdns *cdns, u8 set_active);
>>>> +int cdns_resume(struct cdns *cdns);
>>>>    int cdns_suspend(struct cdns *cdns);
>>>> +void cdns_set_active(struct cdns *cdns, u8 set_active);
>>>>    #else /* CONFIG_PM_SLEEP */
>>>>    static inline int cdns_resume(struct cdns *cdns, u8 set_active)
>>>>    { return 0; }
>>>> --
>>>> 2.25.1
>>>>
>>> Hi,
>>>
>>> This is the friendly semi-automated patch-bot of Greg Kroah-Hartman.
>>> You have sent him a patch that has triggered this response.
>>>
>>> Right now, the development tree you have sent a patch for is "closed"
>>> due to the timing of the merge window.  Don't worry, the patch(es) you
>>> have sent are not lost, and will be looked at after the merge window is
>>> over (after the -rc1 kernel is released by Linus).
>>>
>>> So thank you for your patience and your patches will be reviewed at this
>>> later time, you do not have to do anything further, this is just a short
>>> note to let you know the patch status and so you don't worry they didn't
>>> make it through.
>> Can someone help review this patch?
> It breaks the build:
>
> drivers/usb/cdns3/core.c: In function ‘cdns_resume’:
> drivers/usb/cdns3/core.c:527:24: error: unused variable ‘dev’ [-Werror=unused-variable]
>    527 |         struct device *dev = cdns->dev;
>        |                        ^~~
>
>
> How did you test it?

Oh, sorry, this may be because I did not set the compilation parameters,

which caused me to ignore this warning. I will delete the unused code 
and send the v2 version


thanks

xiaolei

>
> thanks,
>
> greg k-h