Hi,
On 8/23/2023 1:16 PM, Neil Armstrong wrote:
Hi,
On 23/08/2023 07:35, Maulik Shah (mkshah) wrote:
Hi Neil,
@@ -142,8 +163,17 @@ static int qcom_pdc_gic_set_type(struct irq_data
*d, unsigned int type)
return -EINVAL;
}
- old_pdc_type = pdc_reg_read(IRQ_i_CFG, d->hwirq);
- pdc_reg_write(IRQ_i_CFG, d->hwirq, pdc_type);
+ if (pdc_version < PDC_VERSION_3_2) {
+ old_pdc_type = pdc_reg_read(IRQ_i_CFG, d->hwirq);
+ pdc_reg_write(IRQ_i_CFG, d->hwirq, pdc_type);
+ } else {
+ u32 val;
+
+ val = pdc_reg_read(IRQ_i_CFG, d->hwirq);
+ old_pdc_type = val & IRQ_i_CFG_TYPE_MASK;
+ pdc_reg_write(IRQ_i_CFG, d->hwirq,
+ pdc_type | (val & IRQ_i_CFG_IRQ_ENABLE));
+ }
While above is correct, i don't think we need version check in
qcom_pdc_gic_set_type() as bits 0-2 are always for the type in
old/new version as mentioned in v1.
Adding one line after reading old_pdc_type should be good enough.
Yes I understood, but while looking at the IRQ_i_CFG bits, I wanted to
keep the original
driver behavior intact by setting remaining bits to 0.
Adding this single line changes that behavior and keeps bits 3-31
to the default register value, which may have some consequences.
If you consider it's an ok change, then I'll reduce it to this single
line.
Yes this ok change to have single line and should not have any
consequences.
Thanks,
Maulik
