diff mbox series

[1/3] arm64: dts: qcom: sm8350: Remove second reg from pdc

Message ID 1615958996-31807-1-git-send-email-mkshah@codeaurora.org (mailing list archive)
State Rejected
Headers show
Series [1/3] arm64: dts: qcom: sm8350: Remove second reg from pdc | expand

Commit Message

Maulik Shah March 17, 2021, 5:29 a.m. UTC
PDC interrupt controller driver do not use second reg. Remove it.
This is in preparation to convert PDC bindings to yaml where
dtbs_check reports it as additional reg.

Cc: devicetree@vger.kernel.org
Signed-off-by: Maulik Shah <mkshah@codeaurora.org>
---
 arch/arm64/boot/dts/qcom/sm8350.dtsi | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Marc Zyngier March 17, 2021, 9:17 a.m. UTC | #1
On Wed, 17 Mar 2021 05:29:54 +0000,
Maulik Shah <mkshah@codeaurora.org> wrote:
> 
> PDC interrupt controller driver do not use second reg. Remove it.

This is a DT file, not a driver. What the driver does is irrelevant.

The real question is: what does this range do?

Thanks,

	M.
Maulik Shah March 17, 2021, 9:48 a.m. UTC | #2
Hi Marc,

On 3/17/2021 2:47 PM, Marc Zyngier wrote:
> On Wed, 17 Mar 2021 05:29:54 +0000,
> Maulik Shah <mkshah@codeaurora.org> wrote:
>> PDC interrupt controller driver do not use second reg. Remove it.
> This is a DT file, not a driver. What the driver does is irrelevant.
>
> The real question is: what does this range do?
>
> Thanks,
>
> 	M.

This is to set interrupt type in SPI config for which there was a change 
[1] but has not gone in for upstream PDC driver.

The second reg is not used in upstream PDC driver, probably when posting 
downstream DT changes for sm8350/sm8250 it was carried in device node as is.

As its not mentioned in bindigs as well, dtbs_check reports it as 
additional reg when converted to yaml.

[1] 
https://patchwork.kernel.org/project/linux-arm-msm/patch/1568411962-1022-8-git-send-email-ilina@codeaurora.org/

Thanks,
Maulik
>
Marc Zyngier March 17, 2021, 2:02 p.m. UTC | #3
On Wed, 17 Mar 2021 09:48:09 +0000,
Maulik Shah <mkshah@codeaurora.org> wrote:
> 
> Hi Marc,
> 
> On 3/17/2021 2:47 PM, Marc Zyngier wrote:
> > On Wed, 17 Mar 2021 05:29:54 +0000,
> > Maulik Shah <mkshah@codeaurora.org> wrote:
> >> PDC interrupt controller driver do not use second reg. Remove it.
> > This is a DT file, not a driver. What the driver does is irrelevant.
> > 
> > The real question is: what does this range do?
> > 
> > Thanks,
> > 
> > 	M.
> 
> This is to set interrupt type in SPI config for which there was a
> change [1] but has not gone in for upstream PDC driver.
> 
> The second reg is not used in upstream PDC driver, probably when
> posting downstream DT changes for sm8350/sm8250 it was carried in
> device node as is.
> 
> As its not mentioned in bindigs as well, dtbs_check reports it as
> additional reg when converted to yaml.

Then I'd rather you provide accurate documentation in the binding
rather than changing the DT files. Other operating systems may use it,
and it isn't unlikely that Linux could use the feature at some point.

Thanks,

	M.
Bjorn Andersson March 17, 2021, 6:08 p.m. UTC | #4
On Wed 17 Mar 09:02 CDT 2021, Marc Zyngier wrote:

> On Wed, 17 Mar 2021 09:48:09 +0000,
> Maulik Shah <mkshah@codeaurora.org> wrote:
> > 
> > Hi Marc,
> > 
> > On 3/17/2021 2:47 PM, Marc Zyngier wrote:
> > > On Wed, 17 Mar 2021 05:29:54 +0000,
> > > Maulik Shah <mkshah@codeaurora.org> wrote:
> > >> PDC interrupt controller driver do not use second reg. Remove it.
> > > This is a DT file, not a driver. What the driver does is irrelevant.
> > > 
> > > The real question is: what does this range do?
> > > 
> > > Thanks,
> > > 
> > > 	M.
> > 
> > This is to set interrupt type in SPI config for which there was a
> > change [1] but has not gone in for upstream PDC driver.
> > 
> > The second reg is not used in upstream PDC driver, probably when
> > posting downstream DT changes for sm8350/sm8250 it was carried in
> > device node as is.
> > 
> > As its not mentioned in bindigs as well, dtbs_check reports it as
> > additional reg when converted to yaml.
> 
> Then I'd rather you provide accurate documentation in the binding
> rather than changing the DT files. Other operating systems may use it,
> and it isn't unlikely that Linux could use the feature at some point.
> 

I agree. Maulik, please update the DT binding to document this region as
well.


It also seems relevant to pursue getting [1] into the upstream Linux
kernel. Is this something that you use downstream Maulik?

Regards,
Bjorn
Maulik Shah March 22, 2021, 10:26 a.m. UTC | #5
Hi,

On 3/17/2021 11:38 PM, Bjorn Andersson wrote:
> On Wed 17 Mar 09:02 CDT 2021, Marc Zyngier wrote:
>
>> On Wed, 17 Mar 2021 09:48:09 +0000,
>> Maulik Shah <mkshah@codeaurora.org> wrote:
>>> Hi Marc,
>>>
>>> On 3/17/2021 2:47 PM, Marc Zyngier wrote:
>>>> On Wed, 17 Mar 2021 05:29:54 +0000,
>>>> Maulik Shah <mkshah@codeaurora.org> wrote:
>>>>> PDC interrupt controller driver do not use second reg. Remove it.
>>>> This is a DT file, not a driver. What the driver does is irrelevant.
>>>>
>>>> The real question is: what does this range do?
>>>>
>>>> Thanks,
>>>>
>>>> 	M.
>>> This is to set interrupt type in SPI config for which there was a
>>> change [1] but has not gone in for upstream PDC driver.
>>>
>>> The second reg is not used in upstream PDC driver, probably when
>>> posting downstream DT changes for sm8350/sm8250 it was carried in
>>> device node as is.
>>>
>>> As its not mentioned in bindigs as well, dtbs_check reports it as
>>> additional reg when converted to yaml.
>> Then I'd rather you provide accurate documentation in the binding
>> rather than changing the DT files. Other operating systems may use it,
>> and it isn't unlikely that Linux could use the feature at some point.
>>
> I agree. Maulik, please update the DT binding to document this region as
> well.
sure. updated in v2.
>
>
> It also seems relevant to pursue getting [1] into the upstream Linux
> kernel. Is this something that you use downstream Maulik?

Yes its used in downstream. We can pursue to get [1] in.

Thanks,
Maulik

>
> Regards,
> Bjorn
diff mbox series

Patch

diff --git a/arch/arm64/boot/dts/qcom/sm8350.dtsi b/arch/arm64/boot/dts/qcom/sm8350.dtsi
index ea28514..b48bc1b 100644
--- a/arch/arm64/boot/dts/qcom/sm8350.dtsi
+++ b/arch/arm64/boot/dts/qcom/sm8350.dtsi
@@ -612,7 +612,7 @@ 
 
 		pdc: interrupt-controller@b220000 {
 			compatible = "qcom,sm8350-pdc", "qcom,pdc";
-			reg = <0 0x0b220000 0 0x30000>, <0 0x17c000f0 0 0x60>;
+			reg = <0 0x0b220000 0 0x30000>;
 			qcom,pdc-ranges = <0 480 40>, <40 140 14>, <54 263 1>,   <55 306 4>,
 					  <59 312 3>, <62 374 2>,  <64 434 2>,   <66 438 3>,
 					  <69 86 1>,  <70 520 54>, <124 609 31>, <155 63 1>,