Messages in this thread | | | Date | Wed, 23 Aug 2023 13:55:53 +0530 | Subject | Re: [PATCH v2] irqchip/qcom-pdc: add support for v3.2 HW | From | "Maulik Shah (mkshah)" <> |
| |
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
| |