Re: [PATCH V3 3/8] soc: qcom-geni-se: Add interconnect support to fix earlycon crash

[Akash Asthana <akashast@codeaurora.org>]

Hi Matthias

On 4/2/2020 1:16 AM, Matthias Kaehlcke wrote:
> On Tue, Mar 31, 2020 at 11:24:57AM -0700, Matthias Kaehlcke wrote:
>> Hi Akash,
>>
>> On Tue, Mar 31, 2020 at 04:39:31PM +0530, Akash Asthana wrote:
>>> QUP core clock is shared among all the SE drivers present on particular
>>> QUP wrapper, the system will reset(unclocked access) if earlycon used after
>>> QUP core clock is put to 0 from other SE drivers before real console comes
>>> up.
>>>
>>> As earlycon can't vote for it's QUP core need, to fix this add ICC
>>> support to common/QUP wrapper driver and put vote for QUP core from
>>> probe on behalf of earlycon and remove vote during earlycon exit call.
>>>
>>> Signed-off-by: Akash Asthana <akashast@codeaurora.org>
>>> Reported-by: Matthias Kaehlcke <mka@chromium.org>
>>> ---
>>> Change is V3:
>>>   - Add geni_remove_earlycon_icc_vote API that will be used by earlycon
>>>     exit function to remove ICC vote for earlyconsole.
>>>   - Remove suspend/resume hook for geni-se driver as we are no longer
>>>     removing earlyconsole ICC vote from system suspend, we are removing
>>>     from earlycon exit.
>>>
>>>   drivers/soc/qcom/qcom-geni-se.c       | 51 +++++++++++++++++++++++++++++++++++
>>>   drivers/tty/serial/qcom_geni_serial.c |  7 +++++
>>>   include/linux/qcom-geni-se.h          |  2 ++
>>>   3 files changed, 60 insertions(+)
>>>
>>> diff --git a/drivers/soc/qcom/qcom-geni-se.c b/drivers/soc/qcom/qcom-geni-se.c
>>> index 9344c14..d30c282 100644
>>> --- a/drivers/soc/qcom/qcom-geni-se.c
>>> +++ b/drivers/soc/qcom/qcom-geni-se.c
>>> @@ -90,8 +90,11 @@ struct geni_wrapper {
>>>   	struct device *dev;
>>>   	void __iomem *base;
>>>   	struct clk_bulk_data ahb_clks[NUM_AHB_CLKS];
>>> +	struct geni_icc_path to_core;
>>>   };
>>>   
>>> +struct geni_wrapper *earlycon_wrapper;
>> should be static
Yeah ok, I missed it.
>>
>>> +
>>>   #define QUP_HW_VER_REG			0x4
>>>   
>>>   /* Common SE registers */
>>> @@ -818,6 +821,26 @@ int geni_icc_vote_off(struct geni_se *se)
>>>   }
>>>   EXPORT_SYMBOL(geni_icc_vote_off);
>>>   
>>> +void geni_remove_earlycon_icc_vote(void)
>>> +{
>>> +	struct geni_wrapper *wrapper = earlycon_wrapper;
>>> +	struct device_node *parent = of_get_next_parent(wrapper->dev->of_node);
>>> +	struct device_node *child;
>>> +
>>> +	for_each_child_of_node(parent, child) {
>>> +		if (of_device_is_compatible(child, "qcom,geni-se-qup")) {
>>> +			wrapper = platform_get_drvdata(of_find_device_by_node(
>>> +					child));
>>> +			icc_put(wrapper->to_core.path);
>>> +			wrapper->to_core.path = NULL;
>>> +		}
>>> +	}
>>> +	of_node_put(parent);
>>> +
>>> +	earlycon_wrapper = NULL;
>>> +}
>>> +EXPORT_SYMBOL(geni_remove_earlycon_icc_vote);
>> I didn't know that consoles have an exit handler, this is way nicer than
>> the miscellaneous triggers we discussed earlier :)
> No wonder I 'missed' this when looking at the console code for possible
> triggers, it is brand new and as of now only exists in -next:
>
> commit ed31685c96e18f773ca11dd1a637974d62130673
> Author: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Date:   Mon Feb 3 15:31:30 2020 +0200
>
>      console: Introduce ->exit() callback
>
>
> sharp timing!

Yeah this is added recently, even I was not aware of it, Bjorn suggested 
me to use this. Indeed sharp timing!  :)

Regards,

Akash

-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,\na Linux Foundation Collaborative Project