Messages in this thread | | | Date | Wed, 03 Jun 2020 15:54:17 +0530 | From | Sai Prakash Ranjan <> | Subject | Re: [PATCH 2/2] coresight: tmc: Add shutdown callback for TMC ETR/ETF |
| |
Hi Mike,
Thanks again for looking at this.
On 2020-06-03 03:42, Mike Leach wrote: [...]
>> >> SMMU/IOMMU won't be able to do much here as it is the client's >> responsiblity to >> properly shutdown and SMMU device link just makes sure that >> SMMU(supplier) shutdown is >> called only after its consumers shutdown callbacks are called. > > I think this use case can be handled slightly differently than the > general requirements for modular CoreSight drivers. > > What is needed here is a way of stopping the underlying ETR hardware > from issuing data to the SMMU, until the entire device has been shut > down, in a way that does not remove the driver, breaking existing > references and causing a system crash. > > We could introduce a new mode to the ETR driver - e.g. > CS_MODE_SHUTDOWN. > > At the end of the block tmc_shutdown(struct amba_device *adev), set > drvdata->mode to CS_MODE_SHUTDOWN & remove the coresight_unregister(). > This new mode can be used to prevent the underlying hardware from > being able to restart until the device is re-powered. > > This mode can be detected in the code that enables / disables the ETR > and handled appropriately (updates to tmc_enable_etr_sink and > tmc_disable_etr_sink). > This mode will persist until the device is re-started - but because we > are on the device shutdown path this is not an issue. > > This should leave the CoreSight infrastructure stable until the > drivers are shut down normally as part of the device power down > process. >
Sounds good to me, but if the coresight_unregister() is the trouble point causing these crashes, then can't we just remove that from tmc_shutdown() callback? This would be like maintaining the same behaviour as now where on reboot/shutdown we basically don't do anything except for disabling ETR. This way, we do not have to introduce any new mode as well. To be exact, in tmc_shutdown() we just disable ETR and then return without unregistering which should not cause any issues since this is shutdown not the remove callback which is a requirement for making coresight modular like below:
static void tmc_shutdown(struct amba_device *adev) { unsigned long flags; struct tmc_drvdata *drvdata = amba_get_drvdata(adev);
spin_lock_irqsave(&drvdata->spinlock, flags);
if (drvdata->mode == CS_MODE_DISABLED) goto out;
if (drvdata->config_type == TMC_CONFIG_TYPE_ETR) tmc_etr_disable_hw(drvdata);
/* * We do not care about coresight unregister here unlike remove * callback which is required for making coresight modular since * the system is going down after this. */ out: spin_unlock_irqrestore(&drvdata->spinlock, flags); }
Thanks, Sai
-- QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation
| |