Re: [PATCH] usb: dwc3: Add shutdown to platform_driver

From: Felipe Balbi <balbi@kernel•org>
To: Vicente Bergas <vicencb@gmail•com>, Robin Murphy <robin.murphy@arm•com>
Cc: Matthias Brugger <mbrugger@suse•com>,
	Heiko Stuebner <heiko@sntech•de>,
	MarcZyngier <marc.zyngier@arm•com>,
	Catalin Marinas <catalin.marinas@arm•com>,
	linux-usb@vger•kernel.org, Will Deacon <will.deacon@arm•com>,
	linux-rockchip@lists•infradead.org,
	Greg Kroah-Hartman <gregkh@linuxfoundation•org>,
	linux-arm-kernel@lists•infradead.org
Subject: Re: [PATCH] usb: dwc3: Add shutdown to platform_driver
Date: Tue, 27 Aug 2019 14:53:04 +0300	[thread overview]
Message-ID: <87v9uix1sv.fsf@gmail.com> (raw)
In-Reply-To: <8d48017a-64c5-4b25-8d85-113ffcf502c9@gmail.com>

[-- Attachment #1.1: Type: text/plain, Size: 1852 bytes --]

Hi,

Vicente Bergas <vicencb@gmail•com> writes:
> On Saturday, August 17, 2019 7:41:40 PM CEST, Vicente Bergas wrote:
>> Otherwise the device keeps writing to memory after kexec and disturbs
>> the next kernel.
>>
>> Signed-off-by: Vicente Bergas <vicencb@gmail•com>
>> ---
>>  drivers/usb/dwc3/dwc3-of-simple.c | 6 ++++++
>>  1 file changed, 6 insertions(+)
>>
>> Hi Felipe, Robin,
>> this version calls 'remove' from 'shutdown' instead of just asserting
>> a reset because it looks like a cleaner way to stop the device.
>>
>> Calling remove from shutdown in core.c instead of dwc3-of-simple.c does not
>> fix the issue either.
>>
>> It has been tested on the sapphire board, a RK3399 platform.
>>
>> Regards,
>>   Vicenç.
>>
>> diff --git a/drivers/usb/dwc3/dwc3-of-simple.c 
>> b/drivers/usb/dwc3/dwc3-of-simple.c
>> index bdac3e7d7b18..d5fd45c64901 100644
>> --- a/drivers/usb/dwc3/dwc3-of-simple.c
>> +++ b/drivers/usb/dwc3/dwc3-of-simple.c
>> @@ -133,6 +133,11 @@ static int dwc3_of_simple_remove(struct 
>> platform_device *pdev)
>>  	return 0;
>>  }
>>  
>> +static void dwc3_of_simple_shutdown(struct platform_device *pdev)
>> +{
>> +	dwc3_of_simple_remove(pdev);
>> +}
>> +
>>  static int __maybe_unused 
>> dwc3_of_simple_runtime_suspend(struct device *dev)
>>  {
>>  	struct dwc3_of_simple	*simple = dev_get_drvdata(dev);
>> @@ -190,6 +195,7 @@ MODULE_DEVICE_TABLE(of, of_dwc3_simple_match);
>>  static struct platform_driver dwc3_of_simple_driver = {
>>  	.probe		= dwc3_of_simple_probe,
>>  	.remove		= dwc3_of_simple_remove,
>> +	.shutdown	= dwc3_of_simple_shutdown,

why don't you just have shutdown use the same exact function as remove?
Frankly, though, I still don't fully understand what's going wrong
here. Why is the device still alive during kexec?

cheers

-- 
balbi

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists•infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel