mbox series

[00/11] Initial support for the Fairphone 5 smartphone

Message ID 20230830-fp5-initial-v1-0-5a954519bbad@fairphone.com
Headers show
Series Initial support for the Fairphone 5 smartphone | expand

Message

Luca Weiss Aug. 30, 2023, 9:58 a.m. UTC
Add support to boot up mainline kernel on the QCM6490-based Fairphone 5
smartphone.

These patches only cover a part of the functionality brought up on
mainline so far, with the rest needing larger dts and driver changes or
depend on patches that are not yet merged. I will work on sending those
once these base patches here have settled.

Since QCM6490, like SC7280 are 'yupik' in the vendor-provided kernel, we
can base the dts on it and leverage existing support. Though current
sc7280 support mostly assumes ChromeOS devices which have a different
TrustZone setup, so we need to move some ChromeOS-specific bits to the
sc7280-chrome-common.dtsi file to make it boot on a standard TZ board.

Depends on (just for the #include in sc7280.dtsi):
https://lore.kernel.org/linux-arm-msm/20230818-qcom-vmid-defines-v1-1-45b610c96b13@fairphone.com/

The pm7250b patch has been picked up from this series:
https://lore.kernel.org/linux-arm-msm/20230407-pm7250b-sid-v1-2-fc648478cc25@fairphone.com/

Signed-off-by: Luca Weiss <luca.weiss@fairphone.com>
---
Luca Weiss (11):
      arm64: dts: qcom: sc7280: Mark some nodes as 'reserved'
      nvmem: qfprom: Mark core clk as optional
      arm64: dts: qcom: sc7280: Move qfprom clock to chrome-common
      arm64: dts: qcom: pm7250b: make SID configurable
      arm64: dts: qcom: pm8350c: Add flash led node
      dt-bindings: pinctrl: qcom,sc7280: Allow gpio-reserved-ranges
      dt-bindings: arm: qcom,ids: Add SoC ID for QCM6490
      soc: qcom: socinfo: Add SoC ID for QCM6490
      cpufreq: Add QCM6490 to cpufreq-dt-platdev blocklist
      dt-bindings: arm: qcom: Add QCM6490 Fairphone 5
      arm64: dts: qcom: qcm6490: Add device-tree for Fairphone 5

 Documentation/devicetree/bindings/arm/qcom.yaml    |   6 +
 .../bindings/pinctrl/qcom,sc7280-pinctrl.yaml      |   4 +
 arch/arm64/boot/dts/qcom/Makefile                  |   1 +
 arch/arm64/boot/dts/qcom/pm7250b.dtsi              |  23 +-
 arch/arm64/boot/dts/qcom/pm8350c.dtsi              |   6 +
 arch/arm64/boot/dts/qcom/qcm6490-fairphone-fp5.dts | 659 +++++++++++++++++++++
 arch/arm64/boot/dts/qcom/sc7280-chrome-common.dtsi |  17 +
 arch/arm64/boot/dts/qcom/sc7280.dtsi               |   7 +-
 drivers/cpufreq/cpufreq-dt-platdev.c               |   1 +
 drivers/nvmem/qfprom.c                             |   2 +-
 drivers/soc/qcom/socinfo.c                         |   1 +
 include/dt-bindings/arm/qcom,ids.h                 |   1 +
 12 files changed, 717 insertions(+), 11 deletions(-)
---
base-commit: 0255bba921438ea1e45d3f0873c3e8c5a1e03876
change-id: 20230818-fp5-initial-b6c8210ba9c8

Best regards,

Comments

Konrad Dybcio Aug. 30, 2023, 10:45 a.m. UTC | #1
On 30.08.2023 12:35, Luca Weiss wrote:
> On Wed Aug 30, 2023 at 12:08 PM CEST, Konrad Dybcio wrote:
>> On 30.08.2023 11:58, Luca Weiss wrote:
>>> With the standard Qualcomm TrustZone setup, components such as lpasscc,
>>> pdc_reset and watchdog shouldn't be touched by Linux. Mark them with
>>> the status 'reserved' and reeable them in the chrome-common dtsi.
>>>
>>> Signed-off-by: Luca Weiss <luca.weiss@fairphone.com>
>>> ---
>> Could probably use /* Owned by ADSP firmware */ or /* Owned by Gunyah hyp */
> 
> Do you know which one is more fitting for these nodes? I don't really
> have a reference to if the ADSP or Gunyah (is this even used here?) owns
> this.
ADSP owns the audio hw, Gunyah owns the wdog

Konrad
Linus Walleij Aug. 30, 2023, 11:47 a.m. UTC | #2
On Wed, Aug 30, 2023 at 11:58 AM Luca Weiss <luca.weiss@fairphone.com> wrote:

> Allow the gpio-reserved-ranges property on SC7280 TLMM.
>
> Signed-off-by: Luca Weiss <luca.weiss@fairphone.com>

Acked-by: Linus Walleij <linus.walleij@linaro.org>

I assume this will be merged with the rest of the patches, poke me
after v6.6-rc1 if you want me to apply it to the pinctrl tree.

Yours,
Linus Walleij
Luca Weiss Aug. 31, 2023, 10:12 a.m. UTC | #3
On Wed Aug 30, 2023 at 12:06 PM CEST, Krzysztof Kozlowski wrote:
> On 30/08/2023 11:58, Luca Weiss wrote:
> > Like other Qualcomm PMICs the PM7250B can be used on different addresses
> > on the SPMI bus. Use similar defines like the PMK8350 to make this
> > possible.
> > 
> > Signed-off-by: Luca Weiss <luca.weiss@fairphone.com>
> > ---
> >  arch/arm64/boot/dts/qcom/pm7250b.dtsi | 23 ++++++++++++++++-------
> >  1 file changed, 16 insertions(+), 7 deletions(-)
> > 
> > diff --git a/arch/arm64/boot/dts/qcom/pm7250b.dtsi b/arch/arm64/boot/dts/qcom/pm7250b.dtsi
> > index e8540c36bd99..3514de536baa 100644
> > --- a/arch/arm64/boot/dts/qcom/pm7250b.dtsi
> > +++ b/arch/arm64/boot/dts/qcom/pm7250b.dtsi
> > @@ -7,6 +7,15 @@
> >  #include <dt-bindings/interrupt-controller/irq.h>
> >  #include <dt-bindings/spmi/spmi.h>
> >  
> > +/* This PMIC can be configured to be at different SIDs */
> > +#ifndef PM7250B_SID
> > +	#define PM7250B_SID 2
> > +#endif
>
> Why do you send the same patch as v1, without any reference to previous
> discussions?
>
> You got here feedback already.
>
> https://lore.kernel.org/linux-arm-msm/f52524da-719b-790f-ad2c-0c3f313d9fe9@linaro.org/

Hi Krzysztof,

I did mention that original patch in the cover letter of this series.
I'm definitely aware of the discussion earlier this year there but also
tried to get an update lately if there's any update with no response.

If you want to block this patch, I'll have to remove pm7250b from the
device dts, so we'll lose some functionality. Not sure what other
approaches there could be.

Regards
Luca

>
> Best regards,
> Krzysztof
Dmitry Baryshkov Aug. 31, 2023, 11:33 a.m. UTC | #4
On Thu, 31 Aug 2023 at 13:13, Luca Weiss <luca.weiss@fairphone.com> wrote:
>
> On Wed Aug 30, 2023 at 12:06 PM CEST, Krzysztof Kozlowski wrote:
> > On 30/08/2023 11:58, Luca Weiss wrote:
> > > Like other Qualcomm PMICs the PM7250B can be used on different addresses
> > > on the SPMI bus. Use similar defines like the PMK8350 to make this
> > > possible.
> > >
> > > Signed-off-by: Luca Weiss <luca.weiss@fairphone.com>
> > > ---
> > >  arch/arm64/boot/dts/qcom/pm7250b.dtsi | 23 ++++++++++++++++-------
> > >  1 file changed, 16 insertions(+), 7 deletions(-)
> > >
> > > diff --git a/arch/arm64/boot/dts/qcom/pm7250b.dtsi b/arch/arm64/boot/dts/qcom/pm7250b.dtsi
> > > index e8540c36bd99..3514de536baa 100644
> > > --- a/arch/arm64/boot/dts/qcom/pm7250b.dtsi
> > > +++ b/arch/arm64/boot/dts/qcom/pm7250b.dtsi
> > > @@ -7,6 +7,15 @@
> > >  #include <dt-bindings/interrupt-controller/irq.h>
> > >  #include <dt-bindings/spmi/spmi.h>
> > >
> > > +/* This PMIC can be configured to be at different SIDs */
> > > +#ifndef PM7250B_SID
> > > +   #define PM7250B_SID 2
> > > +#endif
> >
> > Why do you send the same patch as v1, without any reference to previous
> > discussions?
> >
> > You got here feedback already.
> >
> > https://lore.kernel.org/linux-arm-msm/f52524da-719b-790f-ad2c-0c3f313d9fe9@linaro.org/
>
> Hi Krzysztof,
>
> I did mention that original patch in the cover letter of this series.
> I'm definitely aware of the discussion earlier this year there but also
> tried to get an update lately if there's any update with no response.

I think the overall consensus was that my proposal is too complicated
for the DT files.

>
> If you want to block this patch, I'll have to remove pm7250b from the
> device dts, so we'll lose some functionality. Not sure what other
> approaches there could be.
>
> Regards
> Luca
>
> >
> > Best regards,
> > Krzysztof
>
Krzysztof Kozlowski Aug. 31, 2023, 11:54 a.m. UTC | #5
On 31/08/2023 13:33, Dmitry Baryshkov wrote:
> On Thu, 31 Aug 2023 at 13:13, Luca Weiss <luca.weiss@fairphone.com> wrote:
>>
>> On Wed Aug 30, 2023 at 12:06 PM CEST, Krzysztof Kozlowski wrote:
>>> On 30/08/2023 11:58, Luca Weiss wrote:
>>>> Like other Qualcomm PMICs the PM7250B can be used on different addresses
>>>> on the SPMI bus. Use similar defines like the PMK8350 to make this
>>>> possible.
>>>>
>>>> Signed-off-by: Luca Weiss <luca.weiss@fairphone.com>
>>>> ---
>>>>  arch/arm64/boot/dts/qcom/pm7250b.dtsi | 23 ++++++++++++++++-------
>>>>  1 file changed, 16 insertions(+), 7 deletions(-)
>>>>
>>>> diff --git a/arch/arm64/boot/dts/qcom/pm7250b.dtsi b/arch/arm64/boot/dts/qcom/pm7250b.dtsi
>>>> index e8540c36bd99..3514de536baa 100644
>>>> --- a/arch/arm64/boot/dts/qcom/pm7250b.dtsi
>>>> +++ b/arch/arm64/boot/dts/qcom/pm7250b.dtsi
>>>> @@ -7,6 +7,15 @@
>>>>  #include <dt-bindings/interrupt-controller/irq.h>
>>>>  #include <dt-bindings/spmi/spmi.h>
>>>>
>>>> +/* This PMIC can be configured to be at different SIDs */
>>>> +#ifndef PM7250B_SID
>>>> +   #define PM7250B_SID 2
>>>> +#endif
>>>
>>> Why do you send the same patch as v1, without any reference to previous
>>> discussions?
>>>
>>> You got here feedback already.
>>>
>>> https://lore.kernel.org/linux-arm-msm/f52524da-719b-790f-ad2c-0c3f313d9fe9@linaro.org/
>>
>> Hi Krzysztof,
>>
>> I did mention that original patch in the cover letter of this series.
>> I'm definitely aware of the discussion earlier this year there but also
>> tried to get an update lately if there's any update with no response.
> 
> I think the overall consensus was that my proposal is too complicated
> for the DT files.

I proposed to duplicate the entries. Do you keep QUP nodes in DTSI and
customize per address? No.

I definitely do not agree to these ifndef->define. Maybe using just
define would work (so drop ifndef->define), because this makes it
obvious and fail-safe if included in wrong place... except that it is
still not the define we expect. This is not the coding style present in
other DTSes.

The true problem how these SPMI bindings were created. Requiring SID
address in every child is clearly redundant and I think we do not follow
such approach anywhere else.

Best regards,
Krzysztof
Dmitry Baryshkov Aug. 31, 2023, 12:27 p.m. UTC | #6
On Thu, 31 Aug 2023 at 14:54, Krzysztof Kozlowski
<krzysztof.kozlowski@linaro.org> wrote:
>
> On 31/08/2023 13:33, Dmitry Baryshkov wrote:
> > On Thu, 31 Aug 2023 at 13:13, Luca Weiss <luca.weiss@fairphone.com> wrote:
> >>
> >> On Wed Aug 30, 2023 at 12:06 PM CEST, Krzysztof Kozlowski wrote:
> >>> On 30/08/2023 11:58, Luca Weiss wrote:
> >>>> Like other Qualcomm PMICs the PM7250B can be used on different addresses
> >>>> on the SPMI bus. Use similar defines like the PMK8350 to make this
> >>>> possible.
> >>>>
> >>>> Signed-off-by: Luca Weiss <luca.weiss@fairphone.com>
> >>>> ---
> >>>>  arch/arm64/boot/dts/qcom/pm7250b.dtsi | 23 ++++++++++++++++-------
> >>>>  1 file changed, 16 insertions(+), 7 deletions(-)
> >>>>
> >>>> diff --git a/arch/arm64/boot/dts/qcom/pm7250b.dtsi b/arch/arm64/boot/dts/qcom/pm7250b.dtsi
> >>>> index e8540c36bd99..3514de536baa 100644
> >>>> --- a/arch/arm64/boot/dts/qcom/pm7250b.dtsi
> >>>> +++ b/arch/arm64/boot/dts/qcom/pm7250b.dtsi
> >>>> @@ -7,6 +7,15 @@
> >>>>  #include <dt-bindings/interrupt-controller/irq.h>
> >>>>  #include <dt-bindings/spmi/spmi.h>
> >>>>
> >>>> +/* This PMIC can be configured to be at different SIDs */
> >>>> +#ifndef PM7250B_SID
> >>>> +   #define PM7250B_SID 2
> >>>> +#endif
> >>>
> >>> Why do you send the same patch as v1, without any reference to previous
> >>> discussions?
> >>>
> >>> You got here feedback already.
> >>>
> >>> https://lore.kernel.org/linux-arm-msm/f52524da-719b-790f-ad2c-0c3f313d9fe9@linaro.org/
> >>
> >> Hi Krzysztof,
> >>
> >> I did mention that original patch in the cover letter of this series.
> >> I'm definitely aware of the discussion earlier this year there but also
> >> tried to get an update lately if there's any update with no response.
> >
> > I think the overall consensus was that my proposal is too complicated
> > for the DT files.
>
> I proposed to duplicate the entries. Do you keep QUP nodes in DTSI and
> customize per address? No.

At the same time, we do keep SoC files separate from the board files.
Yes, I'm slightly exaggerating here.

I think that for PMIC files it makes sense to extract common parts if
that eases reuse of the common parts.

>
> I definitely do not agree to these ifndef->define. Maybe using just
> define would work (so drop ifndef->define), because this makes it
> obvious and fail-safe if included in wrong place... except that it is
> still not the define we expect. This is not the coding style present in
> other DTSes.
>
> The true problem how these SPMI bindings were created. Requiring SID
> address in every child is clearly redundant and I think we do not follow
> such approach anywhere else.
>
> Best regards,
> Krzysztof
>
Luca Weiss Aug. 31, 2023, 12:32 p.m. UTC | #7
On Thu Aug 31, 2023 at 1:54 PM CEST, Krzysztof Kozlowski wrote:
> On 31/08/2023 13:33, Dmitry Baryshkov wrote:
> > On Thu, 31 Aug 2023 at 13:13, Luca Weiss <luca.weiss@fairphone.com> wrote:
> >>
> >> On Wed Aug 30, 2023 at 12:06 PM CEST, Krzysztof Kozlowski wrote:
> >>> On 30/08/2023 11:58, Luca Weiss wrote:
> >>>> Like other Qualcomm PMICs the PM7250B can be used on different addresses
> >>>> on the SPMI bus. Use similar defines like the PMK8350 to make this
> >>>> possible.
> >>>>
> >>>> Signed-off-by: Luca Weiss <luca.weiss@fairphone.com>
> >>>> ---
> >>>>  arch/arm64/boot/dts/qcom/pm7250b.dtsi | 23 ++++++++++++++++-------
> >>>>  1 file changed, 16 insertions(+), 7 deletions(-)
> >>>>
> >>>> diff --git a/arch/arm64/boot/dts/qcom/pm7250b.dtsi b/arch/arm64/boot/dts/qcom/pm7250b.dtsi
> >>>> index e8540c36bd99..3514de536baa 100644
> >>>> --- a/arch/arm64/boot/dts/qcom/pm7250b.dtsi
> >>>> +++ b/arch/arm64/boot/dts/qcom/pm7250b.dtsi
> >>>> @@ -7,6 +7,15 @@
> >>>>  #include <dt-bindings/interrupt-controller/irq.h>
> >>>>  #include <dt-bindings/spmi/spmi.h>
> >>>>
> >>>> +/* This PMIC can be configured to be at different SIDs */
> >>>> +#ifndef PM7250B_SID
> >>>> +   #define PM7250B_SID 2
> >>>> +#endif
> >>>
> >>> Why do you send the same patch as v1, without any reference to previous
> >>> discussions?
> >>>
> >>> You got here feedback already.
> >>>
> >>> https://lore.kernel.org/linux-arm-msm/f52524da-719b-790f-ad2c-0c3f313d9fe9@linaro.org/
> >>
> >> Hi Krzysztof,
> >>
> >> I did mention that original patch in the cover letter of this series.
> >> I'm definitely aware of the discussion earlier this year there but also
> >> tried to get an update lately if there's any update with no response.
> > 
> > I think the overall consensus was that my proposal is too complicated
> > for the DT files.
>
> I proposed to duplicate the entries.

If you mean creating a pm7250b-8.dtsi with pm7250b copy-pasted but the
SID changed from 2 & 3 to 8 & 9, I can do that if that's the way
forward.

If this was done, I'd also say then that pm7250b.dtsi should be renamed
to e.g. pm7250b-2.dtsi since it's currently sitting on SID 2 & 3.

> Do you keep QUP nodes in DTSI and customize per address? No.
>
> I definitely do not agree to these ifndef->define. Maybe using just
> define would work (so drop ifndef->define), because this makes it
> obvious and fail-safe if included in wrong place... except that it is
> still not the define we expect. This is not the coding style present in
> other DTSes.

I really don't mind either way, I'd just like to have some way for now.

>
> The true problem how these SPMI bindings were created. Requiring SID
> address in every child is clearly redundant and I think we do not follow
> such approach anywhere else.

Is this something that could be fixed long term? Especially since
Qualcomm is reconfiguring PMICs on different addresses nowadays maybe
there's more or a push to do this?

Regards
Luca

>
> Best regards,
> Krzysztof
Luca Weiss Sept. 5, 2023, 8:30 a.m. UTC | #8
On Thu Aug 31, 2023 at 2:27 PM CEST, Dmitry Baryshkov wrote:
> On Thu, 31 Aug 2023 at 14:54, Krzysztof Kozlowski
> <krzysztof.kozlowski@linaro.org> wrote:
> >
> > On 31/08/2023 13:33, Dmitry Baryshkov wrote:
> > > On Thu, 31 Aug 2023 at 13:13, Luca Weiss <luca.weiss@fairphone.com> wrote:
> > >>
> > >> On Wed Aug 30, 2023 at 12:06 PM CEST, Krzysztof Kozlowski wrote:
> > >>> On 30/08/2023 11:58, Luca Weiss wrote:
> > >>>> Like other Qualcomm PMICs the PM7250B can be used on different addresses
> > >>>> on the SPMI bus. Use similar defines like the PMK8350 to make this
> > >>>> possible.
> > >>>>
> > >>>> Signed-off-by: Luca Weiss <luca.weiss@fairphone.com>
> > >>>> ---
> > >>>>  arch/arm64/boot/dts/qcom/pm7250b.dtsi | 23 ++++++++++++++++-------
> > >>>>  1 file changed, 16 insertions(+), 7 deletions(-)
> > >>>>
> > >>>> diff --git a/arch/arm64/boot/dts/qcom/pm7250b.dtsi b/arch/arm64/boot/dts/qcom/pm7250b.dtsi
> > >>>> index e8540c36bd99..3514de536baa 100644
> > >>>> --- a/arch/arm64/boot/dts/qcom/pm7250b.dtsi
> > >>>> +++ b/arch/arm64/boot/dts/qcom/pm7250b.dtsi
> > >>>> @@ -7,6 +7,15 @@
> > >>>>  #include <dt-bindings/interrupt-controller/irq.h>
> > >>>>  #include <dt-bindings/spmi/spmi.h>
> > >>>>
> > >>>> +/* This PMIC can be configured to be at different SIDs */
> > >>>> +#ifndef PM7250B_SID
> > >>>> +   #define PM7250B_SID 2
> > >>>> +#endif
> > >>>
> > >>> Why do you send the same patch as v1, without any reference to previous
> > >>> discussions?
> > >>>
> > >>> You got here feedback already.
> > >>>
> > >>> https://lore.kernel.org/linux-arm-msm/f52524da-719b-790f-ad2c-0c3f313d9fe9@linaro.org/
> > >>
> > >> Hi Krzysztof,
> > >>
> > >> I did mention that original patch in the cover letter of this series.
> > >> I'm definitely aware of the discussion earlier this year there but also
> > >> tried to get an update lately if there's any update with no response.
> > >
> > > I think the overall consensus was that my proposal is too complicated
> > > for the DT files.
> >
> > I proposed to duplicate the entries. Do you keep QUP nodes in DTSI and
> > customize per address? No.
>
> At the same time, we do keep SoC files separate from the board files.
> Yes, I'm slightly exaggerating here.
>
> I think that for PMIC files it makes sense to extract common parts if
> that eases reuse of the common parts.

Hi all,

what can I do for v2 now?

1. Keep this patch as-is, and keep pm7250b in device dts.

2. Drop pm7250b patch and drop from device dts, until _someone_ figures
out a solution talking to the PMIC on different SID.

3. Something else like copy-pasting pm7250b.dtsi to pm7250-8.dtsi and
changing the SID there, and using that in device dts.

Please let me know what to do.

Regards
Luca

>
> >
> > I definitely do not agree to these ifndef->define. Maybe using just
> > define would work (so drop ifndef->define), because this makes it
> > obvious and fail-safe if included in wrong place... except that it is
> > still not the define we expect. This is not the coding style present in
> > other DTSes.
> >
> > The true problem how these SPMI bindings were created. Requiring SID
> > address in every child is clearly redundant and I think we do not follow
> > such approach anywhere else.
> >
> > Best regards,
> > Krzysztof
> >
Luca Weiss Sept. 11, 2023, 8:34 a.m. UTC | #9
On Tue Sep 5, 2023 at 10:30 AM CEST, Luca Weiss wrote:
> On Thu Aug 31, 2023 at 2:27 PM CEST, Dmitry Baryshkov wrote:
> > On Thu, 31 Aug 2023 at 14:54, Krzysztof Kozlowski
> > <krzysztof.kozlowski@linaro.org> wrote:
> > >
> > > On 31/08/2023 13:33, Dmitry Baryshkov wrote:
> > > > On Thu, 31 Aug 2023 at 13:13, Luca Weiss <luca.weiss@fairphone.com> wrote:
> > > >>
> > > >> On Wed Aug 30, 2023 at 12:06 PM CEST, Krzysztof Kozlowski wrote:
> > > >>> On 30/08/2023 11:58, Luca Weiss wrote:
> > > >>>> Like other Qualcomm PMICs the PM7250B can be used on different addresses
> > > >>>> on the SPMI bus. Use similar defines like the PMK8350 to make this
> > > >>>> possible.
> > > >>>>
> > > >>>> Signed-off-by: Luca Weiss <luca.weiss@fairphone.com>
> > > >>>> ---
> > > >>>>  arch/arm64/boot/dts/qcom/pm7250b.dtsi | 23 ++++++++++++++++-------
> > > >>>>  1 file changed, 16 insertions(+), 7 deletions(-)
> > > >>>>
> > > >>>> diff --git a/arch/arm64/boot/dts/qcom/pm7250b.dtsi b/arch/arm64/boot/dts/qcom/pm7250b.dtsi
> > > >>>> index e8540c36bd99..3514de536baa 100644
> > > >>>> --- a/arch/arm64/boot/dts/qcom/pm7250b.dtsi
> > > >>>> +++ b/arch/arm64/boot/dts/qcom/pm7250b.dtsi
> > > >>>> @@ -7,6 +7,15 @@
> > > >>>>  #include <dt-bindings/interrupt-controller/irq.h>
> > > >>>>  #include <dt-bindings/spmi/spmi.h>
> > > >>>>
> > > >>>> +/* This PMIC can be configured to be at different SIDs */
> > > >>>> +#ifndef PM7250B_SID
> > > >>>> +   #define PM7250B_SID 2
> > > >>>> +#endif
> > > >>>
> > > >>> Why do you send the same patch as v1, without any reference to previous
> > > >>> discussions?
> > > >>>
> > > >>> You got here feedback already.
> > > >>>
> > > >>> https://lore.kernel.org/linux-arm-msm/f52524da-719b-790f-ad2c-0c3f313d9fe9@linaro.org/
> > > >>
> > > >> Hi Krzysztof,
> > > >>
> > > >> I did mention that original patch in the cover letter of this series.
> > > >> I'm definitely aware of the discussion earlier this year there but also
> > > >> tried to get an update lately if there's any update with no response.
> > > >
> > > > I think the overall consensus was that my proposal is too complicated
> > > > for the DT files.
> > >
> > > I proposed to duplicate the entries. Do you keep QUP nodes in DTSI and
> > > customize per address? No.
> >
> > At the same time, we do keep SoC files separate from the board files.
> > Yes, I'm slightly exaggerating here.
> >
> > I think that for PMIC files it makes sense to extract common parts if
> > that eases reuse of the common parts.
>
> Hi all,
>
> what can I do for v2 now?
>
> 1. Keep this patch as-is, and keep pm7250b in device dts.
>
> 2. Drop pm7250b patch and drop from device dts, until _someone_ figures
> out a solution talking to the PMIC on different SID.
>
> 3. Something else like copy-pasting pm7250b.dtsi to pm7250-8.dtsi and
> changing the SID there, and using that in device dts.
>
> Please let me know what to do.
>
> Regards
> Luca

Hi,

if there's no feedback I'll keep this patch in v2 of this series and we
can continue to discuss there (if necessary).

Regards
Luca

>
> >
> > >
> > > I definitely do not agree to these ifndef->define. Maybe using just
> > > define would work (so drop ifndef->define), because this makes it
> > > obvious and fail-safe if included in wrong place... except that it is
> > > still not the define we expect. This is not the coding style present in
> > > other DTSes.
> > >
> > > The true problem how these SPMI bindings were created. Requiring SID
> > > address in every child is clearly redundant and I think we do not follow
> > > such approach anywhere else.
> > >
> > > Best regards,
> > > Krzysztof
> > >
Krzysztof Kozlowski Sept. 11, 2023, 9:44 a.m. UTC | #10
On 11/09/2023 10:34, Luca Weiss wrote:
> On Tue Sep 5, 2023 at 10:30 AM CEST, Luca Weiss wrote:
>> On Thu Aug 31, 2023 at 2:27 PM CEST, Dmitry Baryshkov wrote:
>>> On Thu, 31 Aug 2023 at 14:54, Krzysztof Kozlowski
>>> <krzysztof.kozlowski@linaro.org> wrote:
>>>>
>>>> On 31/08/2023 13:33, Dmitry Baryshkov wrote:
>>>>> On Thu, 31 Aug 2023 at 13:13, Luca Weiss <luca.weiss@fairphone.com> wrote:
>>>>>>
>>>>>> On Wed Aug 30, 2023 at 12:06 PM CEST, Krzysztof Kozlowski wrote:
>>>>>>> On 30/08/2023 11:58, Luca Weiss wrote:
>>>>>>>> Like other Qualcomm PMICs the PM7250B can be used on different addresses
>>>>>>>> on the SPMI bus. Use similar defines like the PMK8350 to make this
>>>>>>>> possible.
>>>>>>>>
>>>>>>>> Signed-off-by: Luca Weiss <luca.weiss@fairphone.com>
>>>>>>>> ---
>>>>>>>>  arch/arm64/boot/dts/qcom/pm7250b.dtsi | 23 ++++++++++++++++-------
>>>>>>>>  1 file changed, 16 insertions(+), 7 deletions(-)
>>>>>>>>
>>>>>>>> diff --git a/arch/arm64/boot/dts/qcom/pm7250b.dtsi b/arch/arm64/boot/dts/qcom/pm7250b.dtsi
>>>>>>>> index e8540c36bd99..3514de536baa 100644
>>>>>>>> --- a/arch/arm64/boot/dts/qcom/pm7250b.dtsi
>>>>>>>> +++ b/arch/arm64/boot/dts/qcom/pm7250b.dtsi
>>>>>>>> @@ -7,6 +7,15 @@
>>>>>>>>  #include <dt-bindings/interrupt-controller/irq.h>
>>>>>>>>  #include <dt-bindings/spmi/spmi.h>
>>>>>>>>
>>>>>>>> +/* This PMIC can be configured to be at different SIDs */
>>>>>>>> +#ifndef PM7250B_SID
>>>>>>>> +   #define PM7250B_SID 2
>>>>>>>> +#endif
>>>>>>>
>>>>>>> Why do you send the same patch as v1, without any reference to previous
>>>>>>> discussions?
>>>>>>>
>>>>>>> You got here feedback already.
>>>>>>>
>>>>>>> https://lore.kernel.org/linux-arm-msm/f52524da-719b-790f-ad2c-0c3f313d9fe9@linaro.org/
>>>>>>
>>>>>> Hi Krzysztof,
>>>>>>
>>>>>> I did mention that original patch in the cover letter of this series.
>>>>>> I'm definitely aware of the discussion earlier this year there but also
>>>>>> tried to get an update lately if there's any update with no response.
>>>>>
>>>>> I think the overall consensus was that my proposal is too complicated
>>>>> for the DT files.
>>>>
>>>> I proposed to duplicate the entries. Do you keep QUP nodes in DTSI and
>>>> customize per address? No.
>>>
>>> At the same time, we do keep SoC files separate from the board files.
>>> Yes, I'm slightly exaggerating here.
>>>
>>> I think that for PMIC files it makes sense to extract common parts if
>>> that eases reuse of the common parts.
>>
>> Hi all,
>>
>> what can I do for v2 now?
>>
>> 1. Keep this patch as-is, and keep pm7250b in device dts.

This was NAKed by me. What Qualcomm SoC maintainers decide (or not
decide) about other options, should not cause the wrong solution to be
re-posted...

>>
>> 2. Drop pm7250b patch and drop from device dts, until _someone_ figures
>> out a solution talking to the PMIC on different SID.
>>
>> 3. Something else like copy-pasting pm7250b.dtsi to pm7250-8.dtsi and
>> changing the SID there, and using that in device dts.
>>
>> Please let me know what to do.
>>
>> Regards
>> Luca
> 
> Hi,
> 
> if there's no feedback I'll keep this patch in v2 of this series and we
> can continue to discuss there (if necessary).

Sorry, I still do not agree and there were no arguments convincing me to
change the mind.

I gave you the solution from my perspective. Why do you decided to
ignore it and send it as is?


Best regards,
Krzysztof
Bjorn Andersson Sept. 14, 2023, 4:04 p.m. UTC | #11
On Wed, 30 Aug 2023 11:58:25 +0200, Luca Weiss wrote:
> Add support to boot up mainline kernel on the QCM6490-based Fairphone 5
> smartphone.
> 
> These patches only cover a part of the functionality brought up on
> mainline so far, with the rest needing larger dts and driver changes or
> depend on patches that are not yet merged. I will work on sending those
> once these base patches here have settled.
> 
> [...]

Applied, thanks!

[07/11] dt-bindings: arm: qcom,ids: Add SoC ID for QCM6490
        commit: ccfb4d8b606302d857a03ea29039e21029311335
[08/11] soc: qcom: socinfo: Add SoC ID for QCM6490
        commit: 59872d59d164ec67f295d6f96fe818b92973ee40

Best regards,