Re: [PATCH v3 2/3] arm64: dts: qcom: purwa: Drop the Hamoa workaround for PDC

[Maulik Shah (mkshah)]

On 7/24/2026 12:02 AM, Bjorn Andersson wrote:
> On Fri, Jul 17, 2026 at 09:11:28AM +0530, Maulik Shah (mkshah) wrote:
>>
>>
>> On 7/17/2026 7:38 AM, Bjorn Andersson wrote:
>>> On Thu, Jul 16, 2026 at 09:59:44AM +0200, Krzysztof Kozlowski wrote:
>>>> On Wed, Jul 15, 2026 at 06:52:01PM +0530, Maulik Shah wrote:
>>>>> X1P42100 (Purwa) shares the X1E80100 (Hamoa) PDC device, but the hardware
>>>>> register bug addressed in commit e9a48ea4d90b ("irqchip/qcom-pdc:
>>>>> Workaround hardware register bug on X1E80100") is already fixed in
>>>>> X1P42100 silicon.
>>>>>
>>>>> X1E80100 compatible forces the software workaround. Use the X1P42100
>>>>> specific compatible string for the PDC node to remove the workaround.
>>>>>
>>>>> Fixes: f08edb529916 ("arm64: dts: qcom: Add X1P42100 SoC and CRD")
>>>>> Reviewed-by: Konrad Dybcio <konrad.dybcio@xxxxxxxxxxxxxxxx>
>>>>> Signed-off-by: Maulik Shah <maulik.shah@xxxxxxxxxxxxxxxx>
>>>>> ---
>>>>>  arch/arm64/boot/dts/qcom/purwa.dtsi | 5 +++++
>>>>>  1 file changed, 5 insertions(+)
>>>>
>>>> Why does the DT change appear in the middle of the patchset? Please read
>>>> submitting patches documents - both of them - and maintainer-soc
>>>> profile.
>>>>
>>>
>>> I thought I had figured it out, but I'm not sure anymore.
>>>
>>> The claim from the cover letter is that patch 1 and 2 are completely
>>> independent, but patch 3 depends on Bartosz's thank you letter [2] that
>>> arrived a week before this series was sent out.
>>>
>>> We're not merging the three changes through the same tree and there's no
>>> expressed dependency between patch 2 and 3 (only implicitly by the order
>>> in the series). But as Konrad points out, in-between patch 2 and 3 we
>>> would not enable the secondary GPIO in the PDC driver, so Purwa would
>>> have broken GPIOs for a while (not ok). I think merging them in the
>>> opposite order would be what we want (i.e. 1, 3, then 2)
>>
>> purwa-iot-evk.dts where firmware sets pass through mode, so current order of the patch seems to be ok.
>> (patch 3 as such is no impact for iot evk)
> 
> But to reiterate the critical point, this is going to be merged by two
> different maintainers into two different trees, taking two different
> paths to mainline etc.
> 
> So, to preserve this order, it either need to be extremely clear that
> this order is required and from that maintainers can figure out if it's
> possible to jump through hoops to maintain it.

Sure. Let me update the cover letter with order to be picked up.

> 
>>
>> x1p42100-crd.dts where firmware sets secondary mode, applying in 1, 2, and then 3 may leave crd boards
>> in broken GPIOs for a while after [2],  so yes order 1, 3, and then 2 makes more sense.
>>
>> patch 1/2 - fixes the purwa to operate on correct registers.
>> patch 3 - Allow crd boards to re-set the mode to pass through
>>
> 
> If 1, 3, and then 2 is a valid order then send the patches in that
> order, if 2 must come last, then we need to ensure that the dependencies
> are making it to mainline first - which you can do by resubmitting the
> two parts separately.
Sure, by resubmitting two parts separately, shall i send (patch 1 and patch 3) as first part and
patch-2 as second part? or only changing the order to patch 1-3-2 with updated cover letter mentioning
dependency? 

> 
>>>
>>>
>>> But this series implies that Purwa has been broken from the start - that
>>> the PDC driver has always operated on the wrong registers.
>>
>> yes, purwa always operated on the wrong registers.
>>
> 
> But by accident it we never touched those registers. So, is purwa
> completely broken now that you have gotten the pinctrl wired up?

purwa touched those PDC registers as direct SPIs were already wired up via
respective device nodes in devicetree. 

Change [2] additionally wired up pinctrl/GPIO used as wakeup interrupts too.

> 
>>>
>>> Perhaps the impact of this was limited as there's not that many direct
>>> &pdc references in the DT, but the patch that Bartosz's thank-you email
>>> was sent for got merged as 77fbc756d9cb ("Revert "pinctrl: qcom:
>>> x1e80100: Bypass PDC wakeup parent for now""), and that would make a lot
>>> more use of the PDC.
>>>
>>> So while nothing in this series states it, it sounds like Purwa might be
>>> completely broken right now and this series aims to fix it?
>>>
>>>
>>> It's not clear to me why the driver change doesn't have a Fixes tag, it
>>> seems like the patch that introduced x1e_quirk was broken and should be
>>> marked as Fixes.
>>
>> x1e_quirk in the driver via commit e9a48ea4d90b ("irqchip/qcom-pdc:
>> Workaround hardware register bug on X1E80100") says only about x1e/
>> hamoa specific bug, it seemed it never wanted to enable the quirk for
>> x1p / purwa. the quirk rather got anyway enabled for purwa due to both
>> hamoa/purwa sharing the same compatible.
>>
> 
> This patch series is the first piece of information telling me that
> Purwa did not inherit this hardware bug.
> 
>> Patch 1/2 of the series aims to fix this (and patch-2 carries the
>> fixes: tag).
> 
> So patch 2 should be backported to LTS kernels, but 1 and 3 shouldn't?

LTS kernel do not have pintctrl/GPIOs wired up via PDC so patch 3 is not required to be backported.

patch 2 should be backported to LTS kernel, so purwa operates on the correct registers for
direct SPIs which were already wired up via PDC on LTS kernels.

When 2 is backported (new compatible) its documentation patch 1 would also be required to follow.
however fixes tag is only kept in patch-2 and not in patch-1 as per discussion in [3]).

[3] https://lore.kernel.org/linux-arm-msm/655b8897-b3f6-4a49-95df-fc07c520c5c6@xxxxxxxxxx/

Thanks,
Maulik

> 
> Regards,
> Bjorn
> 
>>
>> Thanks,
>> Maulik
>>
>>>
>>> [2] https://lore.kernel.org/linux-arm-msm/CAMRc=MeU0QuRozMscv02M59+a66S05Jm18CyvNE-qSYrY=S7hQ@xxxxxxxxxxxxxx/
>>>
>>> Regards,
>>> Bjorn
>>>
>>>> Best regards,
>>>> Krzysztof
>>>>
>>