Re: [PATCH 2/2] coresight: tmc: Add shutdown callback for TMC ETR/ETF

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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



[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [Linux for Sparc]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux