On 21/05/2024 21:17, Bjorn Andersson wrote: >>> >>> Hi Krzysztof, >>> >>> Sorry for the confusion, I dont think I meant that the smem driver will >>> ever crash. The referred to crash in the cover letter is a crash in the >>> firmware running on the remoteproc. The remoteproc could crash for any >>> unexpected reason, related or unrelated to smem, while holding the tcsr >>> mutex. I want to ensure that all resources that a remoteproc might be >>> using are released as part of remoteproc stop. >>> >>> The SMEM driver manages the lock/unlock operations on the tcsr mutex >>> from the Linux CPU's perspective. This case is for cleaning up from the >>> remote side's perspective. >>> >>> In this case it's the hwspinlock used to synchronize SMEM, but it's >>> conceivable that firmware running on the remoteproc has additional locks >>> that need to be busted in order for the system to continue executing >>> until the firmware is reinitialized. >>> >>> We did consider tying this to the SMEM instance, but the entitiy >>> relating to firmware is the remoteproc instance. >> >> I still do not understand why you have to add hwlock to remoteproc, even >> though it is not directly used. Your driver problem looks like lack of >> proper driver architecture - you want to control the locks not from the >> layer took the lock, but one layer up. Sorry, no, fix the driver >> architecture. >> > > No, it is the firmware's reference to the lock that is represented in > the remoteproc node, while SMEM deals with Linux's reference to the lock. > > This reference would be used to release the lock - on behalf of the > firmware - in the event that the firmware held it when it > stopped/crashed. I understood, but the remoteproc driver did not acquire the hardware lock. It was taken by smem, if I got it correctly, so you should poke smem to bust the spinlock. The hwlock is not a property of remote proc, because remote proc does not care, right? Other device cares... and now for every smem user you will add new binding property? No, you are adding a binding based on your driver solution. Best regards, Krzysztof
