All of lore.kernel.org
 help / color / mirror / Atom feed
From: Bjorn Andersson <andersson@kernel.org>
To: "Maulik Shah (mkshah)" <maulik.shah@oss.qualcomm.com>
Cc: Krzysztof Kozlowski <krzk@kernel.org>,
	 Thomas Gleixner <tglx@kernel.org>, Rob Herring <robh@kernel.org>,
	 Krzysztof Kozlowski <krzk+dt@kernel.org>,
	Conor Dooley <conor+dt@kernel.org>,
	 Konrad Dybcio <konradybcio@kernel.org>,
	linux-arm-msm@vger.kernel.org, linux-kernel@vger.kernel.org,
	 devicetree@vger.kernel.org,
	Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
Subject: Re: [PATCH v3 2/3] arm64: dts: qcom: purwa: Drop the Hamoa workaround for PDC
Date: Thu, 23 Jul 2026 13:32:19 -0500	[thread overview]
Message-ID: <amJcXDcI0QYqtOnK@baldur> (raw)
In-Reply-To: <64bf3979-51cc-42ed-b66b-c7af04536481@oss.qualcomm.com>

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@oss.qualcomm.com>
> >>> Signed-off-by: Maulik Shah <maulik.shah@oss.qualcomm.com>
> >>> ---
> >>>  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.

> 
> 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.

> > 
> > 
> > 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?

> > 
> > 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?

Regards,
Bjorn

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

  reply	other threads:[~2026-07-23 18:32 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-15 13:21 [PATCH v3 0/3] purwa: Drop the Hamoa PDC workaround from purwa Maulik Shah
2026-07-15 13:22 ` [PATCH v3 1/3] dt-bindings: interrupt-controller: qcom,pdc: Document Purwa PDC Maulik Shah
2026-07-15 13:22 ` [PATCH v3 2/3] arm64: dts: qcom: purwa: Drop the Hamoa workaround for PDC Maulik Shah
2026-07-15 13:39   ` sashiko-bot
2026-07-16  7:59   ` Krzysztof Kozlowski
2026-07-17  2:08     ` Bjorn Andersson
2026-07-17  3:41       ` Maulik Shah (mkshah)
2026-07-23 18:32         ` Bjorn Andersson [this message]
2026-07-15 13:22 ` [PATCH v3 3/3] irqchip/qcom-pdc: Add puwra compatible for PDC secondary mode Maulik Shah
2026-07-15 13:32 ` [PATCH v3 0/3] purwa: Drop the Hamoa PDC workaround from purwa Konrad Dybcio

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=amJcXDcI0QYqtOnK@baldur \
    --to=andersson@kernel.org \
    --cc=conor+dt@kernel.org \
    --cc=devicetree@vger.kernel.org \
    --cc=konrad.dybcio@oss.qualcomm.com \
    --cc=konradybcio@kernel.org \
    --cc=krzk+dt@kernel.org \
    --cc=krzk@kernel.org \
    --cc=linux-arm-msm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=maulik.shah@oss.qualcomm.com \
    --cc=robh@kernel.org \
    --cc=tglx@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.