[PATCH for-6.1.y] Revert "interconnect: qcom: sm8250: Enable sync_state"

[PATCH for-6.1.y] Revert "interconnect: qcom: sm8250: Enable sync_state"

[Amit Pundir]

This reverts commit 3637f6bdfe2ccd53c493836b6e43c9a73e4513b3 which is
commit bfc7db1cb94ad664546d70212699f8cc6c539e8c upstream.

This resulted in boot regression on RB5 (sm8250), causing the device
to hard crash into USB crash dump mode everytime.

Signed-off-by: Amit Pundir <amit.pundir@linaro.org>
---
 drivers/interconnect/qcom/sm8250.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/interconnect/qcom/sm8250.c b/drivers/interconnect/qcom/sm8250.c
index 9c2dd40d9a55..5cdb058fa095 100644
--- a/drivers/interconnect/qcom/sm8250.c
+++ b/drivers/interconnect/qcom/sm8250.c
@@ -551,7 +551,6 @@ static struct platform_driver qnoc_driver = {
 	.driver = {
 		.name = "qnoc-sm8250",
 		.of_match_table = qnoc_of_match,
-		.sync_state = icc_sync_state,
 	},
 };
 module_platform_driver(qnoc_driver);
-- 
2.25.1

Re: [PATCH for-6.1.y] Revert "interconnect: qcom: sm8250: Enable sync_state"

[Greg KH]

On Sun, Jan 07, 2024 at 09:27:02PM +0530, Amit Pundir wrote:
> This reverts commit 3637f6bdfe2ccd53c493836b6e43c9a73e4513b3 which is
> commit bfc7db1cb94ad664546d70212699f8cc6c539e8c upstream.
> 
> This resulted in boot regression on RB5 (sm8250), causing the device
> to hard crash into USB crash dump mode everytime.
> 
> Signed-off-by: Amit Pundir <amit.pundir@linaro.org>

Any link to that report?  Is this also an issue in 6.7 and/or 6.6.y?

thanks,

greg k-h

Re: [PATCH for-6.1.y] Revert "interconnect: qcom: sm8250: Enable sync_state"

[Amit Pundir]

On Mon, 8 Jan 2024 at 19:42, Greg KH <gregkh@linuxfoundation.org> wrote:
>
> On Sun, Jan 07, 2024 at 09:27:02PM +0530, Amit Pundir wrote:
> > This reverts commit 3637f6bdfe2ccd53c493836b6e43c9a73e4513b3 which is
> > commit bfc7db1cb94ad664546d70212699f8cc6c539e8c upstream.
> >
> > This resulted in boot regression on RB5 (sm8250), causing the device
> > to hard crash into USB crash dump mode everytime.
> >
> > Signed-off-by: Amit Pundir <amit.pundir@linaro.org>
>
> Any link to that report?  Is this also an issue in 6.7 and/or 6.6.y?

Here is a fresh RB5 crash report running AOSP with upstream v6.1.71
https://lkft.validation.linaro.org/scheduler/job/7151629#L4239

I do not see this crash on v6.7.

I have not tested v6.6.y yet. I'll test and submit a revert if I see
this crash there as well.

Regards,
Amit Pundir

>
> thanks,
>
> greg k-h
>

Re: [PATCH for-6.1.y] Revert "interconnect: qcom: sm8250: Enable sync_state"

[Greg KH]

On Mon, Jan 08, 2024 at 08:33:00PM +0530, Amit Pundir wrote:
> On Mon, 8 Jan 2024 at 19:42, Greg KH <gregkh@linuxfoundation.org> wrote:
> >
> > On Sun, Jan 07, 2024 at 09:27:02PM +0530, Amit Pundir wrote:
> > > This reverts commit 3637f6bdfe2ccd53c493836b6e43c9a73e4513b3 which is
> > > commit bfc7db1cb94ad664546d70212699f8cc6c539e8c upstream.
> > >
> > > This resulted in boot regression on RB5 (sm8250), causing the device
> > > to hard crash into USB crash dump mode everytime.
> > >
> > > Signed-off-by: Amit Pundir <amit.pundir@linaro.org>
> >
> > Any link to that report?  Is this also an issue in 6.7 and/or 6.6.y?
> 
> Here is a fresh RB5 crash report running AOSP with upstream v6.1.71
> https://lkft.validation.linaro.org/scheduler/job/7151629#L4239
> 
> I do not see this crash on v6.7.

So does that mean we are instead missing something here for this tree?

thanks,

greg k-h

Re: [PATCH for-6.1.y] Revert "interconnect: qcom: sm8250: Enable sync_state"

[Konrad Dybcio]

On 8.01.2024 16:19, Greg KH wrote:
> On Mon, Jan 08, 2024 at 08:33:00PM +0530, Amit Pundir wrote:
>> On Mon, 8 Jan 2024 at 19:42, Greg KH <gregkh@linuxfoundation.org> wrote:
>>>
>>> On Sun, Jan 07, 2024 at 09:27:02PM +0530, Amit Pundir wrote:
>>>> This reverts commit 3637f6bdfe2ccd53c493836b6e43c9a73e4513b3 which is
>>>> commit bfc7db1cb94ad664546d70212699f8cc6c539e8c upstream.
>>>>
>>>> This resulted in boot regression on RB5 (sm8250), causing the device
>>>> to hard crash into USB crash dump mode everytime.
>>>>
>>>> Signed-off-by: Amit Pundir <amit.pundir@linaro.org>
>>>
>>> Any link to that report?  Is this also an issue in 6.7 and/or 6.6.y?
>>
>> Here is a fresh RB5 crash report running AOSP with upstream v6.1.71
>> https://lkft.validation.linaro.org/scheduler/job/7151629#L4239
>>
>> I do not see this crash on v6.7.
> 
> So does that mean we are instead missing something here for this tree?

Yes, however I'm not sure anybody is keen on tracking that down, as
(in short) for the platform to work correctly (.sync_state on
interconnect not crashing the thing into oblivion is one of the
signs), a lot of things need to be in place. And some developers
never validated that properly..

Removing .sync_state from the SoC interconnect driver translates
into "keep the power flowing on all data buses", which helps avoid
crashes that are mainly caused by unclocked accesses and alike.

Konrad

Re: [PATCH for-6.1.y] Revert "interconnect: qcom: sm8250: Enable sync_state"

[Greg KH]

On Mon, Jan 08, 2024 at 04:30:42PM +0100, Konrad Dybcio wrote:
> On 8.01.2024 16:19, Greg KH wrote:
> > On Mon, Jan 08, 2024 at 08:33:00PM +0530, Amit Pundir wrote:
> >> On Mon, 8 Jan 2024 at 19:42, Greg KH <gregkh@linuxfoundation.org> wrote:
> >>>
> >>> On Sun, Jan 07, 2024 at 09:27:02PM +0530, Amit Pundir wrote:
> >>>> This reverts commit 3637f6bdfe2ccd53c493836b6e43c9a73e4513b3 which is
> >>>> commit bfc7db1cb94ad664546d70212699f8cc6c539e8c upstream.
> >>>>
> >>>> This resulted in boot regression on RB5 (sm8250), causing the device
> >>>> to hard crash into USB crash dump mode everytime.
> >>>>
> >>>> Signed-off-by: Amit Pundir <amit.pundir@linaro.org>
> >>>
> >>> Any link to that report?  Is this also an issue in 6.7 and/or 6.6.y?
> >>
> >> Here is a fresh RB5 crash report running AOSP with upstream v6.1.71
> >> https://lkft.validation.linaro.org/scheduler/job/7151629#L4239
> >>
> >> I do not see this crash on v6.7.
> > 
> > So does that mean we are instead missing something here for this tree?
> 
> Yes, however I'm not sure anybody is keen on tracking that down, as
> (in short) for the platform to work correctly (.sync_state on
> interconnect not crashing the thing into oblivion is one of the
> signs), a lot of things need to be in place. And some developers
> never validated that properly..
> 
> Removing .sync_state from the SoC interconnect driver translates
> into "keep the power flowing on all data buses", which helps avoid
> crashes that are mainly caused by unclocked accesses and alike.

Ok, queued up now, thanks.

greg k-h