* [PATCH] qcom: llcc/edac: Correct interrupt enable register configuration
@ 2024-11-19 6:46 Komal Bajaj
2024-11-20 11:59 ` Dmitry Baryshkov
2025-02-14 13:18 ` Manivannan Sadhasivam
0 siblings, 2 replies; 6+ messages in thread
From: Komal Bajaj @ 2024-11-19 6:46 UTC (permalink / raw)
To: manivannan.sadhasivam, bp, tony.luck, james.morse, mchehab, rric,
andy.gross, vnkgutta
Cc: linux-arm-msm, linux-edac, linux-kernel, Komal Bajaj
The previous implementation incorrectly configured the cmn_interrupt_2_enable
register for interrupt handling. Using cmn_interrupt_2_enable to configure Tag,
Data RAM ECC interrupts would lead to issues like double handling of the
interrupts (EL1 and EL3) as cmn_interrupt_2_enable is meant to be configured
for interrupts which needs to be handled by EL3.
EL1 LLCC EDAC driver needs to use cmn_interrupt_0_enable register to
configure Tag, Data RAM ECC interrupts instead of cmn_interrupt_2_enable.
Fixes: 27450653f1db ("drivers: edac: Add EDAC driver support for QCOM SoCs")
Signed-off-by: Komal Bajaj <quic_kbajaj@quicinc.com>
---
drivers/edac/qcom_edac.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/edac/qcom_edac.c b/drivers/edac/qcom_edac.c
index a9a8ba067007..0fd7a777fe7d 100644
--- a/drivers/edac/qcom_edac.c
+++ b/drivers/edac/qcom_edac.c
@@ -95,7 +95,7 @@ static int qcom_llcc_core_setup(struct llcc_drv_data *drv, struct regmap *llcc_b
* Configure interrupt enable registers such that Tag, Data RAM related
* interrupts are propagated to interrupt controller for servicing
*/
- ret = regmap_update_bits(llcc_bcast_regmap, drv->edac_reg_offset->cmn_interrupt_2_enable,
+ ret = regmap_update_bits(llcc_bcast_regmap, drv->edac_reg_offset->cmn_interrupt_0_enable,
TRP0_INTERRUPT_ENABLE,
TRP0_INTERRUPT_ENABLE);
if (ret)
@@ -113,7 +113,7 @@ static int qcom_llcc_core_setup(struct llcc_drv_data *drv, struct regmap *llcc_b
if (ret)
return ret;
- ret = regmap_update_bits(llcc_bcast_regmap, drv->edac_reg_offset->cmn_interrupt_2_enable,
+ ret = regmap_update_bits(llcc_bcast_regmap, drv->edac_reg_offset->cmn_interrupt_0_enable,
DRP0_INTERRUPT_ENABLE,
DRP0_INTERRUPT_ENABLE);
if (ret)
--
2.46.0
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] qcom: llcc/edac: Correct interrupt enable register configuration
2024-11-19 6:46 [PATCH] qcom: llcc/edac: Correct interrupt enable register configuration Komal Bajaj
@ 2024-11-20 11:59 ` Dmitry Baryshkov
2024-12-05 10:29 ` Komal Bajaj
2025-02-14 13:18 ` Manivannan Sadhasivam
1 sibling, 1 reply; 6+ messages in thread
From: Dmitry Baryshkov @ 2024-11-20 11:59 UTC (permalink / raw)
To: Komal Bajaj
Cc: manivannan.sadhasivam, bp, tony.luck, james.morse, mchehab, rric,
andy.gross, vnkgutta, linux-arm-msm, linux-edac, linux-kernel
On Tue, Nov 19, 2024 at 12:16:08PM +0530, Komal Bajaj wrote:
> The previous implementation incorrectly configured the cmn_interrupt_2_enable
> register for interrupt handling. Using cmn_interrupt_2_enable to configure Tag,
> Data RAM ECC interrupts would lead to issues like double handling of the
> interrupts (EL1 and EL3) as cmn_interrupt_2_enable is meant to be configured
> for interrupts which needs to be handled by EL3.
This reads as if it was possible to write EL3-related register from EL1.
Is it true?
>
> EL1 LLCC EDAC driver needs to use cmn_interrupt_0_enable register to
> configure Tag, Data RAM ECC interrupts instead of cmn_interrupt_2_enable.
>
> Fixes: 27450653f1db ("drivers: edac: Add EDAC driver support for QCOM SoCs")
> Signed-off-by: Komal Bajaj <quic_kbajaj@quicinc.com>
> ---
> drivers/edac/qcom_edac.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/edac/qcom_edac.c b/drivers/edac/qcom_edac.c
> index a9a8ba067007..0fd7a777fe7d 100644
> --- a/drivers/edac/qcom_edac.c
> +++ b/drivers/edac/qcom_edac.c
> @@ -95,7 +95,7 @@ static int qcom_llcc_core_setup(struct llcc_drv_data *drv, struct regmap *llcc_b
> * Configure interrupt enable registers such that Tag, Data RAM related
> * interrupts are propagated to interrupt controller for servicing
> */
> - ret = regmap_update_bits(llcc_bcast_regmap, drv->edac_reg_offset->cmn_interrupt_2_enable,
> + ret = regmap_update_bits(llcc_bcast_regmap, drv->edac_reg_offset->cmn_interrupt_0_enable,
> TRP0_INTERRUPT_ENABLE,
> TRP0_INTERRUPT_ENABLE);
> if (ret)
> @@ -113,7 +113,7 @@ static int qcom_llcc_core_setup(struct llcc_drv_data *drv, struct regmap *llcc_b
> if (ret)
> return ret;
>
> - ret = regmap_update_bits(llcc_bcast_regmap, drv->edac_reg_offset->cmn_interrupt_2_enable,
> + ret = regmap_update_bits(llcc_bcast_regmap, drv->edac_reg_offset->cmn_interrupt_0_enable,
> DRP0_INTERRUPT_ENABLE,
> DRP0_INTERRUPT_ENABLE);
> if (ret)
> --
> 2.46.0
>
--
With best wishes
Dmitry
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] qcom: llcc/edac: Correct interrupt enable register configuration
2024-11-20 11:59 ` Dmitry Baryshkov
@ 2024-12-05 10:29 ` Komal Bajaj
2025-02-14 8:30 ` Komal Bajaj
0 siblings, 1 reply; 6+ messages in thread
From: Komal Bajaj @ 2024-12-05 10:29 UTC (permalink / raw)
To: Dmitry Baryshkov
Cc: manivannan.sadhasivam, bp, tony.luck, james.morse, mchehab, rric,
andy.gross, vnkgutta, linux-arm-msm, linux-edac, linux-kernel
On 11/20/2024 5:29 PM, Dmitry Baryshkov wrote:
> On Tue, Nov 19, 2024 at 12:16:08PM +0530, Komal Bajaj wrote:
>> The previous implementation incorrectly configured the cmn_interrupt_2_enable
>> register for interrupt handling. Using cmn_interrupt_2_enable to configure Tag,
>> Data RAM ECC interrupts would lead to issues like double handling of the
>> interrupts (EL1 and EL3) as cmn_interrupt_2_enable is meant to be configured
>> for interrupts which needs to be handled by EL3.
> This reads as if it was possible to write EL3-related register from EL1.
> Is it true?
Both EL1 and EL3 can access the LLCC "cmn_interrupt_2_enable" register,
but configuring the register from both
EL1 & EL3 shouldn't be done as the register is meant to be configured
from EL3. There was a bug in HPG which was
fixed recently on not to configure the register from EL1.
Thanks
Komal
>> EL1 LLCC EDAC driver needs to use cmn_interrupt_0_enable register to
>> configure Tag, Data RAM ECC interrupts instead of cmn_interrupt_2_enable.
>>
>> Fixes: 27450653f1db ("drivers: edac: Add EDAC driver support for QCOM SoCs")
>> Signed-off-by: Komal Bajaj <quic_kbajaj@quicinc.com>
>> ---
>> drivers/edac/qcom_edac.c | 4 ++--
>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/edac/qcom_edac.c b/drivers/edac/qcom_edac.c
>> index a9a8ba067007..0fd7a777fe7d 100644
>> --- a/drivers/edac/qcom_edac.c
>> +++ b/drivers/edac/qcom_edac.c
>> @@ -95,7 +95,7 @@ static int qcom_llcc_core_setup(struct llcc_drv_data *drv, struct regmap *llcc_b
>> * Configure interrupt enable registers such that Tag, Data RAM related
>> * interrupts are propagated to interrupt controller for servicing
>> */
>> - ret = regmap_update_bits(llcc_bcast_regmap, drv->edac_reg_offset->cmn_interrupt_2_enable,
>> + ret = regmap_update_bits(llcc_bcast_regmap, drv->edac_reg_offset->cmn_interrupt_0_enable,
>> TRP0_INTERRUPT_ENABLE,
>> TRP0_INTERRUPT_ENABLE);
>> if (ret)
>> @@ -113,7 +113,7 @@ static int qcom_llcc_core_setup(struct llcc_drv_data *drv, struct regmap *llcc_b
>> if (ret)
>> return ret;
>>
>> - ret = regmap_update_bits(llcc_bcast_regmap, drv->edac_reg_offset->cmn_interrupt_2_enable,
>> + ret = regmap_update_bits(llcc_bcast_regmap, drv->edac_reg_offset->cmn_interrupt_0_enable,
>> DRP0_INTERRUPT_ENABLE,
>> DRP0_INTERRUPT_ENABLE);
>> if (ret)
>> --
>> 2.46.0
>>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] qcom: llcc/edac: Correct interrupt enable register configuration
2024-12-05 10:29 ` Komal Bajaj
@ 2025-02-14 8:30 ` Komal Bajaj
0 siblings, 0 replies; 6+ messages in thread
From: Komal Bajaj @ 2025-02-14 8:30 UTC (permalink / raw)
To: Dmitry Baryshkov
Cc: manivannan.sadhasivam, bp, tony.luck, james.morse, mchehab, rric,
andy.gross, vnkgutta, linux-arm-msm, linux-edac, linux-kernel
On 12/5/2024 3:59 PM, Komal Bajaj wrote:
>
> On 11/20/2024 5:29 PM, Dmitry Baryshkov wrote:
>> On Tue, Nov 19, 2024 at 12:16:08PM +0530, Komal Bajaj wrote:
>>> The previous implementation incorrectly configured the
>>> cmn_interrupt_2_enable
>>> register for interrupt handling. Using cmn_interrupt_2_enable to
>>> configure Tag,
>>> Data RAM ECC interrupts would lead to issues like double handling of
>>> the
>>> interrupts (EL1 and EL3) as cmn_interrupt_2_enable is meant to be
>>> configured
>>> for interrupts which needs to be handled by EL3.
>> This reads as if it was possible to write EL3-related register from EL1.
>> Is it true?
>
>
> Both EL1 and EL3 can access the LLCC "cmn_interrupt_2_enable"
> register, but configuring the register from both
> EL1 & EL3 shouldn't be done as the register is meant to be configured
> from EL3. There was a bug in HPG which was
> fixed recently on not to configure the register from EL1.
>
> Thanks
> Komal
Hi Dmitry,
Does this answer your query, or is there anything else you are looking for?
Thanks
Komal
>
>
>>> EL1 LLCC EDAC driver needs to use cmn_interrupt_0_enable register to
>>> configure Tag, Data RAM ECC interrupts instead of
>>> cmn_interrupt_2_enable.
>>>
>>> Fixes: 27450653f1db ("drivers: edac: Add EDAC driver support for
>>> QCOM SoCs")
>>> Signed-off-by: Komal Bajaj <quic_kbajaj@quicinc.com>
>>> ---
>>> drivers/edac/qcom_edac.c | 4 ++--
>>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/edac/qcom_edac.c b/drivers/edac/qcom_edac.c
>>> index a9a8ba067007..0fd7a777fe7d 100644
>>> --- a/drivers/edac/qcom_edac.c
>>> +++ b/drivers/edac/qcom_edac.c
>>> @@ -95,7 +95,7 @@ static int qcom_llcc_core_setup(struct
>>> llcc_drv_data *drv, struct regmap *llcc_b
>>> * Configure interrupt enable registers such that Tag, Data
>>> RAM related
>>> * interrupts are propagated to interrupt controller for
>>> servicing
>>> */
>>> - ret = regmap_update_bits(llcc_bcast_regmap,
>>> drv->edac_reg_offset->cmn_interrupt_2_enable,
>>> + ret = regmap_update_bits(llcc_bcast_regmap,
>>> drv->edac_reg_offset->cmn_interrupt_0_enable,
>>> TRP0_INTERRUPT_ENABLE,
>>> TRP0_INTERRUPT_ENABLE);
>>> if (ret)
>>> @@ -113,7 +113,7 @@ static int qcom_llcc_core_setup(struct
>>> llcc_drv_data *drv, struct regmap *llcc_b
>>> if (ret)
>>> return ret;
>>>
>>> - ret = regmap_update_bits(llcc_bcast_regmap,
>>> drv->edac_reg_offset->cmn_interrupt_2_enable,
>>> + ret = regmap_update_bits(llcc_bcast_regmap,
>>> drv->edac_reg_offset->cmn_interrupt_0_enable,
>>> DRP0_INTERRUPT_ENABLE,
>>> DRP0_INTERRUPT_ENABLE);
>>> if (ret)
>>> --
>>> 2.46.0
>>>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] qcom: llcc/edac: Correct interrupt enable register configuration
2024-11-19 6:46 [PATCH] qcom: llcc/edac: Correct interrupt enable register configuration Komal Bajaj
2024-11-20 11:59 ` Dmitry Baryshkov
@ 2025-02-14 13:18 ` Manivannan Sadhasivam
2025-02-14 19:45 ` Borislav Petkov
1 sibling, 1 reply; 6+ messages in thread
From: Manivannan Sadhasivam @ 2025-02-14 13:18 UTC (permalink / raw)
To: Komal Bajaj
Cc: bp, tony.luck, james.morse, mchehab, rric, andy.gross, vnkgutta,
linux-arm-msm, linux-edac, linux-kernel
On Tue, Nov 19, 2024 at 12:16:08PM +0530, Komal Bajaj wrote:
> The previous implementation incorrectly configured the cmn_interrupt_2_enable
> register for interrupt handling. Using cmn_interrupt_2_enable to configure Tag,
> Data RAM ECC interrupts would lead to issues like double handling of the
> interrupts (EL1 and EL3) as cmn_interrupt_2_enable is meant to be configured
> for interrupts which needs to be handled by EL3.
>
> EL1 LLCC EDAC driver needs to use cmn_interrupt_0_enable register to
> configure Tag, Data RAM ECC interrupts instead of cmn_interrupt_2_enable.
>
Cc: stable@vger.kernel.org
> Fixes: 27450653f1db ("drivers: edac: Add EDAC driver support for QCOM SoCs")
> Signed-off-by: Komal Bajaj <quic_kbajaj@quicinc.com>
Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
- Mani
> ---
> drivers/edac/qcom_edac.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/edac/qcom_edac.c b/drivers/edac/qcom_edac.c
> index a9a8ba067007..0fd7a777fe7d 100644
> --- a/drivers/edac/qcom_edac.c
> +++ b/drivers/edac/qcom_edac.c
> @@ -95,7 +95,7 @@ static int qcom_llcc_core_setup(struct llcc_drv_data *drv, struct regmap *llcc_b
> * Configure interrupt enable registers such that Tag, Data RAM related
> * interrupts are propagated to interrupt controller for servicing
> */
> - ret = regmap_update_bits(llcc_bcast_regmap, drv->edac_reg_offset->cmn_interrupt_2_enable,
> + ret = regmap_update_bits(llcc_bcast_regmap, drv->edac_reg_offset->cmn_interrupt_0_enable,
> TRP0_INTERRUPT_ENABLE,
> TRP0_INTERRUPT_ENABLE);
> if (ret)
> @@ -113,7 +113,7 @@ static int qcom_llcc_core_setup(struct llcc_drv_data *drv, struct regmap *llcc_b
> if (ret)
> return ret;
>
> - ret = regmap_update_bits(llcc_bcast_regmap, drv->edac_reg_offset->cmn_interrupt_2_enable,
> + ret = regmap_update_bits(llcc_bcast_regmap, drv->edac_reg_offset->cmn_interrupt_0_enable,
> DRP0_INTERRUPT_ENABLE,
> DRP0_INTERRUPT_ENABLE);
> if (ret)
> --
> 2.46.0
>
--
மணிவண்ணன் சதாசிவம்
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] qcom: llcc/edac: Correct interrupt enable register configuration
2025-02-14 13:18 ` Manivannan Sadhasivam
@ 2025-02-14 19:45 ` Borislav Petkov
0 siblings, 0 replies; 6+ messages in thread
From: Borislav Petkov @ 2025-02-14 19:45 UTC (permalink / raw)
To: Manivannan Sadhasivam
Cc: Komal Bajaj, tony.luck, james.morse, mchehab, rric, andy.gross,
vnkgutta, linux-arm-msm, linux-edac, linux-kernel
On Fri, Feb 14, 2025 at 06:48:40PM +0530, Manivannan Sadhasivam wrote:
> On Tue, Nov 19, 2024 at 12:16:08PM +0530, Komal Bajaj wrote:
> > The previous implementation incorrectly configured the cmn_interrupt_2_enable
> > register for interrupt handling. Using cmn_interrupt_2_enable to configure Tag,
> > Data RAM ECC interrupts would lead to issues like double handling of the
> > interrupts (EL1 and EL3) as cmn_interrupt_2_enable is meant to be configured
> > for interrupts which needs to be handled by EL3.
> >
> > EL1 LLCC EDAC driver needs to use cmn_interrupt_0_enable register to
> > configure Tag, Data RAM ECC interrupts instead of cmn_interrupt_2_enable.
> >
>
> Cc: stable@vger.kernel.org
>
> > Fixes: 27450653f1db ("drivers: edac: Add EDAC driver support for QCOM SoCs")
> > Signed-off-by: Komal Bajaj <quic_kbajaj@quicinc.com>
>
> Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
Applied, thanks.
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2025-02-14 19:45 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-11-19 6:46 [PATCH] qcom: llcc/edac: Correct interrupt enable register configuration Komal Bajaj
2024-11-20 11:59 ` Dmitry Baryshkov
2024-12-05 10:29 ` Komal Bajaj
2025-02-14 8:30 ` Komal Bajaj
2025-02-14 13:18 ` Manivannan Sadhasivam
2025-02-14 19:45 ` Borislav Petkov
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®