[PATCH 0/4] Initial support for phyBOARD-Pollux i.MX8MP

[PATCH 0/4] Initial support for phyBOARD-Pollux i.MX8MP

[Teresa Remmet]

Hello,

this patch stack adds initial support for the SoM phyCORE-i.MX8MP
and the carrier board phyBOARD-Pollux.

Teresa

Teresa Remmet (4):
  arm64: defconfig: Enable rv3028 i2c rtc driver
  arm64: defconfig: Enable PCA9532 support
  bindings: arm: fsl: Add PHYTEC i.MX8MP devicetree bindings
  arm64: dts: freescale: Add support for phyBOARD-Pollux-i.MX8MP

 Documentation/devicetree/bindings/arm/fsl.yaml     |   7 +
 arch/arm64/boot/dts/freescale/Makefile             |   1 +
 .../dts/freescale/imx8mp-phyboard-pollux-rdk.dts   |  16 ++
 .../boot/dts/freescale/imx8mp-phyboard-pollux.dtsi | 152 ++++++++++
 .../boot/dts/freescale/imx8mp-phycore-som.dtsi     | 319 +++++++++++++++++++++
 arch/arm64/configs/defconfig                       |   2 +
 6 files changed, 497 insertions(+)
 create mode 100644 arch/arm64/boot/dts/freescale/imx8mp-phyboard-pollux-rdk.dts
 create mode 100644 arch/arm64/boot/dts/freescale/imx8mp-phyboard-pollux.dtsi
 create mode 100644 arch/arm64/boot/dts/freescale/imx8mp-phycore-som.dtsi

-- 
2.7.4

[PATCH 0/4] Initial support for phyBOARD-Pollux i.MX8MP

[Teresa Remmet]

Hello,

this patch stack adds initial support for the SoM phyCORE-i.MX8MP
and the carrier board phyBOARD-Pollux.

Teresa

Teresa Remmet (4):
  arm64: defconfig: Enable rv3028 i2c rtc driver
  arm64: defconfig: Enable PCA9532 support
  bindings: arm: fsl: Add PHYTEC i.MX8MP devicetree bindings
  arm64: dts: freescale: Add support for phyBOARD-Pollux-i.MX8MP

 Documentation/devicetree/bindings/arm/fsl.yaml     |   7 +
 arch/arm64/boot/dts/freescale/Makefile             |   1 +
 .../dts/freescale/imx8mp-phyboard-pollux-rdk.dts   |  16 ++
 .../boot/dts/freescale/imx8mp-phyboard-pollux.dtsi | 152 ++++++++++
 .../boot/dts/freescale/imx8mp-phycore-som.dtsi     | 319 +++++++++++++++++++++
 arch/arm64/configs/defconfig                       |   2 +
 6 files changed, 497 insertions(+)
 create mode 100644 arch/arm64/boot/dts/freescale/imx8mp-phyboard-pollux-rdk.dts
 create mode 100644 arch/arm64/boot/dts/freescale/imx8mp-phyboard-pollux.dtsi
 create mode 100644 arch/arm64/boot/dts/freescale/imx8mp-phycore-som.dtsi

-- 
2.7.4


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 1/4] arm64: defconfig: Enable rv3028 i2c rtc driver

[Teresa Remmet]

Enable rv3028 i2c rtc driver populated on phyBOARD-Pollux-i.MX8M Plus.

Signed-off-by: Teresa Remmet <t.remmet@phytec.de>
---
 arch/arm64/configs/defconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm64/configs/defconfig b/arch/arm64/configs/defconfig
index 5cfe3cf6f2ac..2034fefb3f44 100644
--- a/arch/arm64/configs/defconfig
+++ b/arch/arm64/configs/defconfig
@@ -818,6 +818,7 @@ CONFIG_RTC_DRV_MAX77686=y
 CONFIG_RTC_DRV_RK808=m
 CONFIG_RTC_DRV_PCF85363=m
 CONFIG_RTC_DRV_RX8581=m
+CONFIG_RTC_DRV_RV3028=y
 CONFIG_RTC_DRV_RV8803=m
 CONFIG_RTC_DRV_S5M=y
 CONFIG_RTC_DRV_DS3232=y
-- 
2.7.4

[PATCH 1/4] arm64: defconfig: Enable rv3028 i2c rtc driver

[Teresa Remmet]

Enable rv3028 i2c rtc driver populated on phyBOARD-Pollux-i.MX8M Plus.

Signed-off-by: Teresa Remmet <t.remmet@phytec.de>
---
 arch/arm64/configs/defconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm64/configs/defconfig b/arch/arm64/configs/defconfig
index 5cfe3cf6f2ac..2034fefb3f44 100644
--- a/arch/arm64/configs/defconfig
+++ b/arch/arm64/configs/defconfig
@@ -818,6 +818,7 @@ CONFIG_RTC_DRV_MAX77686=y
 CONFIG_RTC_DRV_RK808=m
 CONFIG_RTC_DRV_PCF85363=m
 CONFIG_RTC_DRV_RX8581=m
+CONFIG_RTC_DRV_RV3028=y
 CONFIG_RTC_DRV_RV8803=m
 CONFIG_RTC_DRV_S5M=y
 CONFIG_RTC_DRV_DS3232=y
-- 
2.7.4


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 1/4] arm64: defconfig: Enable rv3028 i2c rtc driver

[Krzysztof Kozlowski]

On Fri, Dec 04, 2020 at 09:32:59PM +0100, Teresa Remmet wrote:
> Enable rv3028 i2c rtc driver populated on phyBOARD-Pollux-i.MX8M Plus.
> 
> Signed-off-by: Teresa Remmet <t.remmet@phytec.de>
> ---
>  arch/arm64/configs/defconfig | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/arch/arm64/configs/defconfig b/arch/arm64/configs/defconfig
> index 5cfe3cf6f2ac..2034fefb3f44 100644
> --- a/arch/arm64/configs/defconfig
> +++ b/arch/arm64/configs/defconfig
> @@ -818,6 +818,7 @@ CONFIG_RTC_DRV_MAX77686=y
>  CONFIG_RTC_DRV_RK808=m
>  CONFIG_RTC_DRV_PCF85363=m
>  CONFIG_RTC_DRV_RX8581=m
> +CONFIG_RTC_DRV_RV3028=y

This should be a module.

Best regards,
Krzysztof


>  CONFIG_RTC_DRV_RV8803=m
>  CONFIG_RTC_DRV_S5M=y
>  CONFIG_RTC_DRV_DS3232=y
> -- 
> 2.7.4
> 

Re: [PATCH 1/4] arm64: defconfig: Enable rv3028 i2c rtc driver

[Krzysztof Kozlowski]

On Fri, Dec 04, 2020 at 09:32:59PM +0100, Teresa Remmet wrote:
> Enable rv3028 i2c rtc driver populated on phyBOARD-Pollux-i.MX8M Plus.
> 
> Signed-off-by: Teresa Remmet <t.remmet@phytec.de>
> ---
>  arch/arm64/configs/defconfig | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/arch/arm64/configs/defconfig b/arch/arm64/configs/defconfig
> index 5cfe3cf6f2ac..2034fefb3f44 100644
> --- a/arch/arm64/configs/defconfig
> +++ b/arch/arm64/configs/defconfig
> @@ -818,6 +818,7 @@ CONFIG_RTC_DRV_MAX77686=y
>  CONFIG_RTC_DRV_RK808=m
>  CONFIG_RTC_DRV_PCF85363=m
>  CONFIG_RTC_DRV_RX8581=m
> +CONFIG_RTC_DRV_RV3028=y

This should be a module.

Best regards,
Krzysztof


>  CONFIG_RTC_DRV_RV8803=m
>  CONFIG_RTC_DRV_S5M=y
>  CONFIG_RTC_DRV_DS3232=y
> -- 
> 2.7.4
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 1/4] arm64: defconfig: Enable rv3028 i2c rtc driver

[Teresa Remmet]

Am Montag, den 07.12.2020, 13:10 +0100 schrieb Krzysztof Kozlowski:
> On Fri, Dec 04, 2020 at 09:32:59PM +0100, Teresa Remmet wrote:
> > Enable rv3028 i2c rtc driver populated on phyBOARD-Pollux-i.MX8M
> > Plus.
> > 
> > Signed-off-by: Teresa Remmet <t.remmet@phytec.de>
> > ---
> >  arch/arm64/configs/defconfig | 1 +
> >  1 file changed, 1 insertion(+)
> > 
> > diff --git a/arch/arm64/configs/defconfig
> > b/arch/arm64/configs/defconfig
> > index 5cfe3cf6f2ac..2034fefb3f44 100644
> > --- a/arch/arm64/configs/defconfig
> > +++ b/arch/arm64/configs/defconfig
> > @@ -818,6 +818,7 @@ CONFIG_RTC_DRV_MAX77686=y
> >  CONFIG_RTC_DRV_RK808=m
> >  CONFIG_RTC_DRV_PCF85363=m
> >  CONFIG_RTC_DRV_RX8581=m
> > +CONFIG_RTC_DRV_RV3028=y
> 
> This should be a module.

with having this build in the clock out of this rtc will be disabled as
not using during boot up. The clock out is enabled as default and has
to be disabled explicit. But I guess the correct way would be to find a
solution in the rtc driver itself?

Teresa

> 
> Best regards,
> Krzysztof
> 
> 
> >  CONFIG_RTC_DRV_RV8803=m
> >  CONFIG_RTC_DRV_S5M=y
> >  CONFIG_RTC_DRV_DS3232=y
> > -- 
> > 2.7.4
> > 

Re: [PATCH 1/4] arm64: defconfig: Enable rv3028 i2c rtc driver

[Teresa Remmet]

Am Montag, den 07.12.2020, 13:10 +0100 schrieb Krzysztof Kozlowski:
> On Fri, Dec 04, 2020 at 09:32:59PM +0100, Teresa Remmet wrote:
> > Enable rv3028 i2c rtc driver populated on phyBOARD-Pollux-i.MX8M
> > Plus.
> > 
> > Signed-off-by: Teresa Remmet <t.remmet@phytec.de>
> > ---
> >  arch/arm64/configs/defconfig | 1 +
> >  1 file changed, 1 insertion(+)
> > 
> > diff --git a/arch/arm64/configs/defconfig
> > b/arch/arm64/configs/defconfig
> > index 5cfe3cf6f2ac..2034fefb3f44 100644
> > --- a/arch/arm64/configs/defconfig
> > +++ b/arch/arm64/configs/defconfig
> > @@ -818,6 +818,7 @@ CONFIG_RTC_DRV_MAX77686=y
> >  CONFIG_RTC_DRV_RK808=m
> >  CONFIG_RTC_DRV_PCF85363=m
> >  CONFIG_RTC_DRV_RX8581=m
> > +CONFIG_RTC_DRV_RV3028=y
> 
> This should be a module.

with having this build in the clock out of this rtc will be disabled as
not using during boot up. The clock out is enabled as default and has
to be disabled explicit. But I guess the correct way would be to find a
solution in the rtc driver itself?

Teresa

> 
> Best regards,
> Krzysztof
> 
> 
> >  CONFIG_RTC_DRV_RV8803=m
> >  CONFIG_RTC_DRV_S5M=y
> >  CONFIG_RTC_DRV_DS3232=y
> > -- 
> > 2.7.4
> > 


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 1/4] arm64: defconfig: Enable rv3028 i2c rtc driver

[Krzysztof Kozlowski]

On Mon, Dec 07, 2020 at 02:38:51PM +0100, Teresa Remmet wrote:
> Am Montag, den 07.12.2020, 13:10 +0100 schrieb Krzysztof Kozlowski:
> > On Fri, Dec 04, 2020 at 09:32:59PM +0100, Teresa Remmet wrote:
> > > Enable rv3028 i2c rtc driver populated on phyBOARD-Pollux-i.MX8M
> > > Plus.
> > > 
> > > Signed-off-by: Teresa Remmet <t.remmet@phytec.de>
> > > ---
> > >  arch/arm64/configs/defconfig | 1 +
> > >  1 file changed, 1 insertion(+)
> > > 
> > > diff --git a/arch/arm64/configs/defconfig
> > > b/arch/arm64/configs/defconfig
> > > index 5cfe3cf6f2ac..2034fefb3f44 100644
> > > --- a/arch/arm64/configs/defconfig
> > > +++ b/arch/arm64/configs/defconfig
> > > @@ -818,6 +818,7 @@ CONFIG_RTC_DRV_MAX77686=y
> > >  CONFIG_RTC_DRV_RK808=m
> > >  CONFIG_RTC_DRV_PCF85363=m
> > >  CONFIG_RTC_DRV_RX8581=m
> > > +CONFIG_RTC_DRV_RV3028=y
> > 
> > This should be a module.
> 
> with having this build in the clock out of this rtc will be disabled as
> not using during boot up. The clock out is enabled as default and has
> to be disabled explicit. But I guess the correct way would be to find a
> solution in the rtc driver itself?

Regardless whether it is built-in or module, the driver will behave the
same. So either the driver disables the clock, or not. The only
difference will be *when* the action happens.

Choosing it as built-in for the purpose of disabling some clock is not a
proper approach.

The defconfig here serves only a development/debugging/reference purpose.
It's not for production so anyway the effect on disabling some parts is
not that important.

Best regards,
Krzysztof

Re: [PATCH 1/4] arm64: defconfig: Enable rv3028 i2c rtc driver

[Krzysztof Kozlowski]

On Mon, Dec 07, 2020 at 02:38:51PM +0100, Teresa Remmet wrote:
> Am Montag, den 07.12.2020, 13:10 +0100 schrieb Krzysztof Kozlowski:
> > On Fri, Dec 04, 2020 at 09:32:59PM +0100, Teresa Remmet wrote:
> > > Enable rv3028 i2c rtc driver populated on phyBOARD-Pollux-i.MX8M
> > > Plus.
> > > 
> > > Signed-off-by: Teresa Remmet <t.remmet@phytec.de>
> > > ---
> > >  arch/arm64/configs/defconfig | 1 +
> > >  1 file changed, 1 insertion(+)
> > > 
> > > diff --git a/arch/arm64/configs/defconfig
> > > b/arch/arm64/configs/defconfig
> > > index 5cfe3cf6f2ac..2034fefb3f44 100644
> > > --- a/arch/arm64/configs/defconfig
> > > +++ b/arch/arm64/configs/defconfig
> > > @@ -818,6 +818,7 @@ CONFIG_RTC_DRV_MAX77686=y
> > >  CONFIG_RTC_DRV_RK808=m
> > >  CONFIG_RTC_DRV_PCF85363=m
> > >  CONFIG_RTC_DRV_RX8581=m
> > > +CONFIG_RTC_DRV_RV3028=y
> > 
> > This should be a module.
> 
> with having this build in the clock out of this rtc will be disabled as
> not using during boot up. The clock out is enabled as default and has
> to be disabled explicit. But I guess the correct way would be to find a
> solution in the rtc driver itself?

Regardless whether it is built-in or module, the driver will behave the
same. So either the driver disables the clock, or not. The only
difference will be *when* the action happens.

Choosing it as built-in for the purpose of disabling some clock is not a
proper approach.

The defconfig here serves only a development/debugging/reference purpose.
It's not for production so anyway the effect on disabling some parts is
not that important.

Best regards,
Krzysztof


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 1/4] arm64: defconfig: Enable rv3028 i2c rtc driver

[Teresa Remmet]

Hello Krzysztof,

Am Montag, den 07.12.2020, 14:50 +0100 schrieb Krzysztof Kozlowski:
> On Mon, Dec 07, 2020 at 02:38:51PM +0100, Teresa Remmet wrote:
> > Am Montag, den 07.12.2020, 13:10 +0100 schrieb Krzysztof Kozlowski:
> > > On Fri, Dec 04, 2020 at 09:32:59PM +0100, Teresa Remmet wrote:
> > > > Enable rv3028 i2c rtc driver populated on phyBOARD-Pollux-
> > > > i.MX8M
> > > > Plus.
> > > > 
> > > > Signed-off-by: Teresa Remmet <t.remmet@phytec.de>
> > > > ---
> > > >  arch/arm64/configs/defconfig | 1 +
> > > >  1 file changed, 1 insertion(+)
> > > > 
> > > > diff --git a/arch/arm64/configs/defconfig
> > > > b/arch/arm64/configs/defconfig
> > > > index 5cfe3cf6f2ac..2034fefb3f44 100644
> > > > --- a/arch/arm64/configs/defconfig
> > > > +++ b/arch/arm64/configs/defconfig
> > > > @@ -818,6 +818,7 @@ CONFIG_RTC_DRV_MAX77686=y
> > > >  CONFIG_RTC_DRV_RK808=m
> > > >  CONFIG_RTC_DRV_PCF85363=m
> > > >  CONFIG_RTC_DRV_RX8581=m
> > > > +CONFIG_RTC_DRV_RV3028=y
> > > 
> > > This should be a module.
> > 
> > with having this build in the clock out of this rtc will be
> > disabled as
> > not using during boot up. The clock out is enabled as default and
> > has
> > to be disabled explicit. But I guess the correct way would be to
> > find a
> > solution in the rtc driver itself?
> 
> Regardless whether it is built-in or module, the driver will behave
> the
> same. So either the driver disables the clock, or not. The only
> difference will be *when* the action happens.
> 
> Choosing it as built-in for the purpose of disabling some clock is
> not a
> proper approach.
> 
> The defconfig here serves only a development/debugging/reference
> purpose.
> It's not for production so anyway the effect on disabling some parts
> is
> not that important.

I will include this as module in v2.

Thanks,
Teresa

> 
> Best regards,
> Krzysztof
> 

Re: [PATCH 1/4] arm64: defconfig: Enable rv3028 i2c rtc driver

[Teresa Remmet]

Hello Krzysztof,

Am Montag, den 07.12.2020, 14:50 +0100 schrieb Krzysztof Kozlowski:
> On Mon, Dec 07, 2020 at 02:38:51PM +0100, Teresa Remmet wrote:
> > Am Montag, den 07.12.2020, 13:10 +0100 schrieb Krzysztof Kozlowski:
> > > On Fri, Dec 04, 2020 at 09:32:59PM +0100, Teresa Remmet wrote:
> > > > Enable rv3028 i2c rtc driver populated on phyBOARD-Pollux-
> > > > i.MX8M
> > > > Plus.
> > > > 
> > > > Signed-off-by: Teresa Remmet <t.remmet@phytec.de>
> > > > ---
> > > >  arch/arm64/configs/defconfig | 1 +
> > > >  1 file changed, 1 insertion(+)
> > > > 
> > > > diff --git a/arch/arm64/configs/defconfig
> > > > b/arch/arm64/configs/defconfig
> > > > index 5cfe3cf6f2ac..2034fefb3f44 100644
> > > > --- a/arch/arm64/configs/defconfig
> > > > +++ b/arch/arm64/configs/defconfig
> > > > @@ -818,6 +818,7 @@ CONFIG_RTC_DRV_MAX77686=y
> > > >  CONFIG_RTC_DRV_RK808=m
> > > >  CONFIG_RTC_DRV_PCF85363=m
> > > >  CONFIG_RTC_DRV_RX8581=m
> > > > +CONFIG_RTC_DRV_RV3028=y
> > > 
> > > This should be a module.
> > 
> > with having this build in the clock out of this rtc will be
> > disabled as
> > not using during boot up. The clock out is enabled as default and
> > has
> > to be disabled explicit. But I guess the correct way would be to
> > find a
> > solution in the rtc driver itself?
> 
> Regardless whether it is built-in or module, the driver will behave
> the
> same. So either the driver disables the clock, or not. The only
> difference will be *when* the action happens.
> 
> Choosing it as built-in for the purpose of disabling some clock is
> not a
> proper approach.
> 
> The defconfig here serves only a development/debugging/reference
> purpose.
> It's not for production so anyway the effect on disabling some parts
> is
> not that important.

I will include this as module in v2.

Thanks,
Teresa

> 
> Best regards,
> Krzysztof
> 


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 2/4] arm64: defconfig: Enable PCA9532 support

[Teresa Remmet]

Enable i2c led expander PCA9532 module support populated on
phyBOARD-Pollux-i.MX8M Plus.

Signed-off-by: Teresa Remmet <t.remmet@phytec.de>
---
 arch/arm64/configs/defconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm64/configs/defconfig b/arch/arm64/configs/defconfig
index 2034fefb3f44..14b0a83d8d3f 100644
--- a/arch/arm64/configs/defconfig
+++ b/arch/arm64/configs/defconfig
@@ -801,6 +801,7 @@ CONFIG_MMC_SDHCI_AM654=y
 CONFIG_MMC_OWL=y
 CONFIG_NEW_LEDS=y
 CONFIG_LEDS_CLASS=y
+CONFIG_LEDS_PCA9532=m
 CONFIG_LEDS_GPIO=y
 CONFIG_LEDS_PWM=y
 CONFIG_LEDS_SYSCON=y
-- 
2.7.4

[PATCH 2/4] arm64: defconfig: Enable PCA9532 support

[Teresa Remmet]

Enable i2c led expander PCA9532 module support populated on
phyBOARD-Pollux-i.MX8M Plus.

Signed-off-by: Teresa Remmet <t.remmet@phytec.de>
---
 arch/arm64/configs/defconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm64/configs/defconfig b/arch/arm64/configs/defconfig
index 2034fefb3f44..14b0a83d8d3f 100644
--- a/arch/arm64/configs/defconfig
+++ b/arch/arm64/configs/defconfig
@@ -801,6 +801,7 @@ CONFIG_MMC_SDHCI_AM654=y
 CONFIG_MMC_OWL=y
 CONFIG_NEW_LEDS=y
 CONFIG_LEDS_CLASS=y
+CONFIG_LEDS_PCA9532=m
 CONFIG_LEDS_GPIO=y
 CONFIG_LEDS_PWM=y
 CONFIG_LEDS_SYSCON=y
-- 
2.7.4


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 2/4] arm64: defconfig: Enable PCA9532 support

[Krzysztof Kozlowski]

On Fri, Dec 04, 2020 at 09:33:00PM +0100, Teresa Remmet wrote:
> Enable i2c led expander PCA9532 module support populated on
> phyBOARD-Pollux-i.MX8M Plus.
> 
> Signed-off-by: Teresa Remmet <t.remmet@phytec.de>
> ---
>  arch/arm64/configs/defconfig | 1 +
>  1 file changed, 1 insertion(+)
> 

Reviewed-by: Krzysztof Kozlowski <krzk@kernel.org>

Best regards,
Krzysztof

Re: [PATCH 2/4] arm64: defconfig: Enable PCA9532 support

[Krzysztof Kozlowski]

On Fri, Dec 04, 2020 at 09:33:00PM +0100, Teresa Remmet wrote:
> Enable i2c led expander PCA9532 module support populated on
> phyBOARD-Pollux-i.MX8M Plus.
> 
> Signed-off-by: Teresa Remmet <t.remmet@phytec.de>
> ---
>  arch/arm64/configs/defconfig | 1 +
>  1 file changed, 1 insertion(+)
> 

Reviewed-by: Krzysztof Kozlowski <krzk@kernel.org>

Best regards,
Krzysztof

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 3/4] bindings: arm: fsl: Add PHYTEC i.MX8MP devicetree bindings

[Teresa Remmet]

Add devicetree bindings for i.MX8MP based phyCORE-i.MX8MP
and phyBOARD-Pollux RDK.

Signed-off-by: Teresa Remmet <t.remmet@phytec.de>
---
 Documentation/devicetree/bindings/arm/fsl.yaml | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/Documentation/devicetree/bindings/arm/fsl.yaml b/Documentation/devicetree/bindings/arm/fsl.yaml
index 934289446abb..880d93092f37 100644
--- a/Documentation/devicetree/bindings/arm/fsl.yaml
+++ b/Documentation/devicetree/bindings/arm/fsl.yaml
@@ -433,6 +433,13 @@ properties:
               - fsl,imx8mp-evk            # i.MX8MP EVK Board
           - const: fsl,imx8mp
 
+      - description: PHYTEC phyCORE-i.MX8MP SoM based boards
+        items:
+          - enum:
+              - phytec,imx8mp-phycore-som         # phyCORE-i.MX8MP SoM
+              - phytec,imx8mp-phyboard-pollux-rdk # phyBOARD-Pollux RDK
+          - const: fsl,imx8mp
+
       - description: i.MX8MQ based Boards
         items:
           - enum:
-- 
2.7.4

[PATCH 3/4] bindings: arm: fsl: Add PHYTEC i.MX8MP devicetree bindings

[Teresa Remmet]

Add devicetree bindings for i.MX8MP based phyCORE-i.MX8MP
and phyBOARD-Pollux RDK.

Signed-off-by: Teresa Remmet <t.remmet@phytec.de>
---
 Documentation/devicetree/bindings/arm/fsl.yaml | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/Documentation/devicetree/bindings/arm/fsl.yaml b/Documentation/devicetree/bindings/arm/fsl.yaml
index 934289446abb..880d93092f37 100644
--- a/Documentation/devicetree/bindings/arm/fsl.yaml
+++ b/Documentation/devicetree/bindings/arm/fsl.yaml
@@ -433,6 +433,13 @@ properties:
               - fsl,imx8mp-evk            # i.MX8MP EVK Board
           - const: fsl,imx8mp
 
+      - description: PHYTEC phyCORE-i.MX8MP SoM based boards
+        items:
+          - enum:
+              - phytec,imx8mp-phycore-som         # phyCORE-i.MX8MP SoM
+              - phytec,imx8mp-phyboard-pollux-rdk # phyBOARD-Pollux RDK
+          - const: fsl,imx8mp
+
       - description: i.MX8MQ based Boards
         items:
           - enum:
-- 
2.7.4


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 3/4] bindings: arm: fsl: Add PHYTEC i.MX8MP devicetree bindings

[Krzysztof Kozlowski]

On Fri, Dec 04, 2020 at 09:33:01PM +0100, Teresa Remmet wrote:
> Add devicetree bindings for i.MX8MP based phyCORE-i.MX8MP
> and phyBOARD-Pollux RDK.
> 
> Signed-off-by: Teresa Remmet <t.remmet@phytec.de>
> ---
>  Documentation/devicetree/bindings/arm/fsl.yaml | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/arm/fsl.yaml b/Documentation/devicetree/bindings/arm/fsl.yaml
> index 934289446abb..880d93092f37 100644
> --- a/Documentation/devicetree/bindings/arm/fsl.yaml
> +++ b/Documentation/devicetree/bindings/arm/fsl.yaml
> @@ -433,6 +433,13 @@ properties:
>                - fsl,imx8mp-evk            # i.MX8MP EVK Board
>            - const: fsl,imx8mp
>  
> +      - description: PHYTEC phyCORE-i.MX8MP SoM based boards
> +        items:
> +          - enum:
> +              - phytec,imx8mp-phycore-som         # phyCORE-i.MX8MP SoM
> +              - phytec,imx8mp-phyboard-pollux-rdk # phyBOARD-Pollux RDK

These do not look correct. In this way, you should include them in
"i.MX8MP based Boards" entry. Probably you wanted all "const" instead.

Best regards,
Krzysztof


> +          - const: fsl,imx8mp
> +

Re: [PATCH 3/4] bindings: arm: fsl: Add PHYTEC i.MX8MP devicetree bindings

[Krzysztof Kozlowski]

On Fri, Dec 04, 2020 at 09:33:01PM +0100, Teresa Remmet wrote:
> Add devicetree bindings for i.MX8MP based phyCORE-i.MX8MP
> and phyBOARD-Pollux RDK.
> 
> Signed-off-by: Teresa Remmet <t.remmet@phytec.de>
> ---
>  Documentation/devicetree/bindings/arm/fsl.yaml | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/arm/fsl.yaml b/Documentation/devicetree/bindings/arm/fsl.yaml
> index 934289446abb..880d93092f37 100644
> --- a/Documentation/devicetree/bindings/arm/fsl.yaml
> +++ b/Documentation/devicetree/bindings/arm/fsl.yaml
> @@ -433,6 +433,13 @@ properties:
>                - fsl,imx8mp-evk            # i.MX8MP EVK Board
>            - const: fsl,imx8mp
>  
> +      - description: PHYTEC phyCORE-i.MX8MP SoM based boards
> +        items:
> +          - enum:
> +              - phytec,imx8mp-phycore-som         # phyCORE-i.MX8MP SoM
> +              - phytec,imx8mp-phyboard-pollux-rdk # phyBOARD-Pollux RDK

These do not look correct. In this way, you should include them in
"i.MX8MP based Boards" entry. Probably you wanted all "const" instead.

Best regards,
Krzysztof


> +          - const: fsl,imx8mp
> +

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 3/4] bindings: arm: fsl: Add PHYTEC i.MX8MP devicetree bindings

[Teresa Remmet]

Am Montag, den 07.12.2020, 12:59 +0100 schrieb Krzysztof Kozlowski:
> On Fri, Dec 04, 2020 at 09:33:01PM +0100, Teresa Remmet wrote:
> > Add devicetree bindings for i.MX8MP based phyCORE-i.MX8MP
> > and phyBOARD-Pollux RDK.
> > 
> > Signed-off-by: Teresa Remmet <t.remmet@phytec.de>
> > ---
> >  Documentation/devicetree/bindings/arm/fsl.yaml | 7 +++++++
> >  1 file changed, 7 insertions(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/arm/fsl.yaml
> > b/Documentation/devicetree/bindings/arm/fsl.yaml
> > index 934289446abb..880d93092f37 100644
> > --- a/Documentation/devicetree/bindings/arm/fsl.yaml
> > +++ b/Documentation/devicetree/bindings/arm/fsl.yaml
> > @@ -433,6 +433,13 @@ properties:
> >                - fsl,imx8mp-evk            # i.MX8MP EVK Board
> >            - const: fsl,imx8mp
> >  
> > +      - description: PHYTEC phyCORE-i.MX8MP SoM based boards
> > +        items:
> > +          - enum:
> > +              - phytec,imx8mp-phycore-som         # phyCORE-
> > i.MX8MP SoM
> > +              - phytec,imx8mp-phyboard-pollux-rdk # phyBOARD-
> > Pollux RDK
> 
> These do not look correct. In this way, you should include them in
> "i.MX8MP based Boards" entry. Probably you wanted all "const"
> instead.

I will change them to "const" in v2. 

Thank you,
Teresa

> 
> Best regards,
> Krzysztof
> 
> 
> > +          - const: fsl,imx8mp
> > +

Re: [PATCH 3/4] bindings: arm: fsl: Add PHYTEC i.MX8MP devicetree bindings

[Teresa Remmet]

Am Montag, den 07.12.2020, 12:59 +0100 schrieb Krzysztof Kozlowski:
> On Fri, Dec 04, 2020 at 09:33:01PM +0100, Teresa Remmet wrote:
> > Add devicetree bindings for i.MX8MP based phyCORE-i.MX8MP
> > and phyBOARD-Pollux RDK.
> > 
> > Signed-off-by: Teresa Remmet <t.remmet@phytec.de>
> > ---
> >  Documentation/devicetree/bindings/arm/fsl.yaml | 7 +++++++
> >  1 file changed, 7 insertions(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/arm/fsl.yaml
> > b/Documentation/devicetree/bindings/arm/fsl.yaml
> > index 934289446abb..880d93092f37 100644
> > --- a/Documentation/devicetree/bindings/arm/fsl.yaml
> > +++ b/Documentation/devicetree/bindings/arm/fsl.yaml
> > @@ -433,6 +433,13 @@ properties:
> >                - fsl,imx8mp-evk            # i.MX8MP EVK Board
> >            - const: fsl,imx8mp
> >  
> > +      - description: PHYTEC phyCORE-i.MX8MP SoM based boards
> > +        items:
> > +          - enum:
> > +              - phytec,imx8mp-phycore-som         # phyCORE-
> > i.MX8MP SoM
> > +              - phytec,imx8mp-phyboard-pollux-rdk # phyBOARD-
> > Pollux RDK
> 
> These do not look correct. In this way, you should include them in
> "i.MX8MP based Boards" entry. Probably you wanted all "const"
> instead.

I will change them to "const" in v2. 

Thank you,
Teresa

> 
> Best regards,
> Krzysztof
> 
> 
> > +          - const: fsl,imx8mp
> > +


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 4/4] arm64: dts: freescale: Add support for phyBOARD-Pollux-i.MX8MP

[Teresa Remmet]

Add initial support for phyBOARD-Pollux-i.MX8MP.
Supported basic features:
	* eMMC
	* i2c EEPROM
	* i2c RTC
	* i2c LED
	* PMIC
	* debug UART
	* SD card
	* 1Gbit Ethernet (fec)
	* watchdog

Signed-off-by: Teresa Remmet <t.remmet@phytec.de>
---
 arch/arm64/boot/dts/freescale/Makefile             |   1 +
 .../dts/freescale/imx8mp-phyboard-pollux-rdk.dts   |  16 ++
 .../boot/dts/freescale/imx8mp-phyboard-pollux.dtsi | 152 ++++++++++
 .../boot/dts/freescale/imx8mp-phycore-som.dtsi     | 319 +++++++++++++++++++++
 4 files changed, 488 insertions(+)
 create mode 100644 arch/arm64/boot/dts/freescale/imx8mp-phyboard-pollux-rdk.dts
 create mode 100644 arch/arm64/boot/dts/freescale/imx8mp-phyboard-pollux.dtsi
 create mode 100644 arch/arm64/boot/dts/freescale/imx8mp-phycore-som.dtsi

diff --git a/arch/arm64/boot/dts/freescale/Makefile b/arch/arm64/boot/dts/freescale/Makefile
index acfb8af45912..a43b496678be 100644
--- a/arch/arm64/boot/dts/freescale/Makefile
+++ b/arch/arm64/boot/dts/freescale/Makefile
@@ -37,6 +37,7 @@ dtb-$(CONFIG_ARCH_MXC) += imx8mn-evk.dtb
 dtb-$(CONFIG_ARCH_MXC) += imx8mn-ddr4-evk.dtb
 dtb-$(CONFIG_ARCH_MXC) += imx8mn-var-som-symphony.dtb
 dtb-$(CONFIG_ARCH_MXC) += imx8mp-evk.dtb
+dtb-$(CONFIG_ARCH_MXC) += imx8mp-phyboard-pollux-rdk.dtb
 dtb-$(CONFIG_ARCH_MXC) += imx8mq-evk.dtb
 dtb-$(CONFIG_ARCH_MXC) += imx8mq-hummingboard-pulse.dtb
 dtb-$(CONFIG_ARCH_MXC) += imx8mq-librem5-devkit.dtb
diff --git a/arch/arm64/boot/dts/freescale/imx8mp-phyboard-pollux-rdk.dts b/arch/arm64/boot/dts/freescale/imx8mp-phyboard-pollux-rdk.dts
new file mode 100644
index 000000000000..dd64be32c99d
--- /dev/null
+++ b/arch/arm64/boot/dts/freescale/imx8mp-phyboard-pollux-rdk.dts
@@ -0,0 +1,16 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2020 PHYTEC Messtechnik GmbH
+ * Author: Teresa Remmet <t.remmet@phytec.de>
+ */
+
+/dts-v1/;
+
+#include "imx8mp-phycore-som.dtsi"
+#include "imx8mp-phyboard-pollux.dtsi"
+
+/ {
+	model = "PHYTEC phyBOARD-Pollux i.MX8MP";
+	compatible = "phytec,imx8mp-phyboard-pollux-rdk",
+		     "phytec,imx8mp-phycore-som", "fsl,imx8mp";
+};
diff --git a/arch/arm64/boot/dts/freescale/imx8mp-phyboard-pollux.dtsi b/arch/arm64/boot/dts/freescale/imx8mp-phyboard-pollux.dtsi
new file mode 100644
index 000000000000..dbeaa27eb043
--- /dev/null
+++ b/arch/arm64/boot/dts/freescale/imx8mp-phyboard-pollux.dtsi
@@ -0,0 +1,152 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2020 PHYTEC Messtechnik GmbH
+ * Author: Teresa Remmet <t.remmet@phytec.de>
+ */
+
+#include <dt-bindings/leds/leds-pca9532.h>
+#include <dt-bindings/pwm/pwm.h>
+
+/ {
+	chosen {
+		stdout-path = &uart2;
+	};
+
+	reg_usdhc2_vmmc: regulator-usdhc2 {
+		compatible = "regulator-fixed";
+		regulator-name = "VSD_3V3";
+		regulator-min-microvolt = <3300000>;
+		regulator-max-microvolt = <3300000>;
+		gpio = <&gpio2 19 GPIO_ACTIVE_HIGH>;
+		enable-active-high;
+		startup-delay-us = <100>;
+		off-on-delay-us = <12000>;
+	};
+};
+
+&i2c2 {
+	clock-frequency = <400000>;
+	pinctrl-names = "default";
+	pinctrl-0 = <&pinctrl_i2c2>;
+	pinctrl-1 = <&pinctrl_i2c2_gpio>;
+	sda-gpios = <&gpio5 17 (GPIO_ACTIVE_HIGH | GPIO_OPEN_DRAIN)>;
+	scl-gpios = <&gpio5 16 (GPIO_ACTIVE_HIGH | GPIO_OPEN_DRAIN)>;
+	status = "okay";
+
+	eeprom@51 {
+		compatible = "atmel,24c02";
+		reg = <0x51>;
+		pagesize = <16>;
+		status = "okay";
+	};
+
+	leddimmer@62 {
+		compatible = "nxp,pca9533";
+		reg = <0x62>;
+		status = "okay";
+
+		led1 {
+			label = "red:user1";
+			type = <PCA9532_TYPE_LED>;
+		};
+
+		led2 {
+			label = "green:user2";
+			type = <PCA9532_TYPE_LED>;
+		};
+
+		led3 {
+			label = "blue:user3";
+			type = <PCA9532_TYPE_LED>;
+		};
+	};
+};
+
+&snvs_pwrkey {
+	status = "okay";
+};
+
+/* debug console */
+&uart2 {
+	pinctrl-names = "default";
+	pinctrl-0 = <&pinctrl_uart2>;
+	status = "okay";
+};
+
+/* SD-Card */
+&usdhc2 {
+	pinctrl-names = "default", "state_100mhz", "state_200mhz";
+	pinctrl-0 = <&pinctrl_usdhc2>, <&pinctrl_usdhc2_pins>;
+	pinctrl-1 = <&pinctrl_usdhc2_100mhz>, <&pinctrl_usdhc2_pins>;
+	pinctrl-2 = <&pinctrl_usdhc2_200mhz>, <&pinctrl_usdhc2_pins>;
+	cd-gpios = <&gpio2 12 GPIO_ACTIVE_LOW>;
+	vmmc-supply = <&reg_usdhc2_vmmc>;
+	bus-width = <4>;
+	status = "okay";
+};
+
+&iomuxc {
+	pinctrl_i2c2: i2c2grp {
+		fsl,pins = <
+			MX8MP_IOMUXC_I2C2_SCL__I2C2_SCL		0x400001c3
+			MX8MP_IOMUXC_I2C2_SDA__I2C2_SDA		0x400001c3
+		>;
+	};
+
+	pinctrl_i2c2_gpio: i2c2gpiogrp {
+		fsl,pins = <
+			MX8MP_IOMUXC_I2C2_SCL__GPIO5_IO16	0x1e3
+			MX8MP_IOMUXC_I2C2_SDA__GPIO5_IO17	0x1e3
+		>;
+	};
+
+	pinctrl_uart2: uart2grp {
+		fsl,pins = <
+			MX8MP_IOMUXC_UART2_RXD__UART2_DCE_RX	0x49
+			MX8MP_IOMUXC_UART2_TXD__UART2_DCE_TX	0x49
+		>;
+	};
+
+	pinctrl_usdhc2_pins: usdhc2-gpiogrp {
+		fsl,pins = <
+			MX8MP_IOMUXC_SD2_CD_B__GPIO2_IO12	0x1c4
+			MX8MP_IOMUXC_SD2_RESET_B__GPIO2_IO19	0x41
+		>;
+	};
+
+	pinctrl_usdhc2: usdhc2grp {
+		fsl,pins = <
+			MX8MP_IOMUXC_SD2_CLK__USDHC2_CLK	0x190
+			MX8MP_IOMUXC_SD2_CMD__USDHC2_CMD	0x1d0
+			MX8MP_IOMUXC_SD2_DATA0__USDHC2_DATA0	0x1d0
+			MX8MP_IOMUXC_SD2_DATA1__USDHC2_DATA1	0x1d0
+			MX8MP_IOMUXC_SD2_DATA2__USDHC2_DATA2	0x1d0
+			MX8MP_IOMUXC_SD2_DATA3__USDHC2_DATA3	0x1d0
+			MX8MP_IOMUXC_GPIO1_IO04__USDHC2_VSELECT	0xc1
+		>;
+	};
+
+	pinctrl_usdhc2_100mhz: usdhc2-100mhzgrp {
+		fsl,pins = <
+			MX8MP_IOMUXC_SD2_CLK__USDHC2_CLK	0x194
+			MX8MP_IOMUXC_SD2_CMD__USDHC2_CMD	0x1d4
+			MX8MP_IOMUXC_SD2_DATA0__USDHC2_DATA0	0x1d4
+			MX8MP_IOMUXC_SD2_DATA1__USDHC2_DATA1	0x1d4
+			MX8MP_IOMUXC_SD2_DATA2__USDHC2_DATA2	0x1d4
+			MX8MP_IOMUXC_SD2_DATA3__USDHC2_DATA3	0x1d4
+			MX8MP_IOMUXC_GPIO1_IO04__USDHC2_VSELECT	0xc1
+		>;
+	};
+
+	pinctrl_usdhc2_200mhz: usdhc2-200mhzgrp {
+		fsl,pins = <
+			MX8MP_IOMUXC_SD2_CLK__USDHC2_CLK	0x196
+			MX8MP_IOMUXC_SD2_CMD__USDHC2_CMD	0x1d6
+			MX8MP_IOMUXC_SD2_DATA0__USDHC2_DATA0	0x1d6
+			MX8MP_IOMUXC_SD2_DATA1__USDHC2_DATA1	0x1d6
+			MX8MP_IOMUXC_SD2_DATA2__USDHC2_DATA2	0x1d6
+			MX8MP_IOMUXC_SD2_DATA3__USDHC2_DATA3	0x1d6
+			MX8MP_IOMUXC_GPIO1_IO04__USDHC2_VSELECT	0xc1
+		>;
+	};
+};
diff --git a/arch/arm64/boot/dts/freescale/imx8mp-phycore-som.dtsi b/arch/arm64/boot/dts/freescale/imx8mp-phycore-som.dtsi
new file mode 100644
index 000000000000..e1fdfebd8142
--- /dev/null
+++ b/arch/arm64/boot/dts/freescale/imx8mp-phycore-som.dtsi
@@ -0,0 +1,319 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2020 PHYTEC Messtechnik GmbH
+ * Author: Teresa Remmet <t.remmet@phytec.de>
+ */
+
+#include <dt-bindings/net/ti-dp83867.h>
+#include "imx8mp.dtsi"
+
+/ {
+	model = "PHYTEC phyCORE-i.MX8MP";
+	compatible = "phytec,imx8mp-phycore-som", "fsl,imx8mp";
+
+	aliases {
+		rtc0 = &rv3028;
+		rtc1 = &snvs_rtc;
+	};
+
+	memory@40000000 {
+		device_type = "memory";
+		reg = <0x0 0x40000000 0 0x80000000>;
+	};
+
+	rtcclkout: rv3028-clkout {
+		compatible = "fixed-clock";
+		#clock-cells = <0>;
+		clock-frequency = <32768>;
+		clock-output-names = "rv3028-clkout";
+		status = "disabled";
+	};
+};
+
+&A53_0 {
+	cpu-supply = <&buck2>;
+};
+
+&A53_1 {
+	cpu-supply = <&buck2>;
+};
+
+&A53_2 {
+	cpu-supply = <&buck2>;
+};
+
+&A53_3 {
+	cpu-supply = <&buck2>;
+};
+
+/* ethernet 1 */
+&fec {
+	pinctrl-names = "default";
+	pinctrl-0 = <&pinctrl_fec>;
+	phy-mode = "rgmii-id";
+	phy-handle = <&ethphy1>;
+	fsl,magic-packet;
+	status = "okay";
+
+	mdio {
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		ethphy1: ethernet-phy@0 {
+			compatible = "ethernet-phy-ieee802.3-c22";
+			reg = <0>;
+			interrupt-parent = <&gpio1>;
+			interrupts = <15 IRQ_TYPE_EDGE_FALLING>;
+			ti,rx-internal-delay = <DP83867_RGMIIDCTL_2_00_NS>;
+			ti,tx-internal-delay = <DP83867_RGMIIDCTL_2_00_NS>;
+			ti,fifo-depth = <DP83867_PHYCR_FIFO_DEPTH_4_B_NIB>;
+			ti,clk-output-sel = <DP83867_CLK_O_SEL_OFF>;
+			enet-phy-lane-no-swap;
+		};
+	};
+};
+
+&i2c1 {
+	clock-frequency = <400000>;
+	pinctrl-names = "default";
+	pinctrl-0 = <&pinctrl_i2c1>;
+	pinctrl-1 = <&pinctrl_i2c1_gpio>;
+	sda-gpios = <&gpio5 15 (GPIO_ACTIVE_HIGH | GPIO_OPEN_DRAIN)>;
+	scl-gpios = <&gpio5 14 (GPIO_ACTIVE_HIGH | GPIO_OPEN_DRAIN)>;
+	status = "okay";
+
+	pmic: pca9450@25 {
+		reg = <0x25>;
+		compatible = "nxp,pca9450c";
+		pinctrl-names = "default";
+		pinctrl-0 = <&pinctrl_pmic>;
+		interrupt-parent = <&gpio4>;
+		interrupts = <18 IRQ_TYPE_LEVEL_LOW>;
+
+		regulators {
+			#address-cells = <1>;
+			#size-cells = <0>;
+
+			buck1: BUCK1 {
+				reg = <0>;
+				regulator-compatible = "BUCK1";
+				regulator-min-microvolt = <600000>;
+				regulator-max-microvolt = <2187500>;
+				regulator-boot-on;
+				regulator-always-on;
+				regulator-ramp-delay = <3125>;
+			};
+
+			buck2: BUCK2 {
+				reg = <1>;
+				regulator-compatible = "BUCK2";
+				regulator-min-microvolt = <600000>;
+				regulator-max-microvolt = <2187500>;
+				regulator-boot-on;
+				regulator-always-on;
+				regulator-ramp-delay = <3125>;
+			};
+
+			buck4: BUCK4 {
+				reg = <3>;
+				regulator-compatible = "BUCK4";
+				regulator-min-microvolt = <600000>;
+				regulator-max-microvolt = <3400000>;
+				regulator-boot-on;
+				regulator-always-on;
+			};
+
+			buck5: BUCK5 {
+				reg = <4>;
+				regulator-compatible = "BUCK5";
+				regulator-min-microvolt = <600000>;
+				regulator-max-microvolt = <3400000>;
+				regulator-boot-on;
+				regulator-always-on;
+			};
+
+			buck6: BUCK6 {
+				reg = <5>;
+				regulator-compatible = "BUCK6";
+				regulator-min-microvolt = <600000>;
+				regulator-max-microvolt = <3400000>;
+				regulator-boot-on;
+				regulator-always-on;
+			};
+
+			ldo1: LDO1 {
+				reg = <6>;
+				regulator-compatible = "LDO1";
+				regulator-min-microvolt = <1600000>;
+				regulator-max-microvolt = <3300000>;
+				regulator-boot-on;
+				regulator-always-on;
+			};
+
+			ldo2: LDO2 {
+				reg = <7>;
+				regulator-compatible = "LDO2";
+				regulator-min-microvolt = <800000>;
+				regulator-max-microvolt = <1150000>;
+				regulator-boot-on;
+				regulator-always-on;
+			};
+
+			ldo3: LDO3 {
+				reg = <8>;
+				regulator-compatible = "LDO3";
+				regulator-min-microvolt = <800000>;
+				regulator-max-microvolt = <3300000>;
+				regulator-boot-on;
+				regulator-always-on;
+			};
+
+			ldo4: LDO4 {
+				reg = <9>;
+				regulator-compatible = "LDO4";
+				regulator-min-microvolt = <800000>;
+				regulator-max-microvolt = <3300000>;
+				regulator-boot-on;
+				regulator-always-on;
+			};
+
+			ldo5: LDO5 {
+				reg = <10>;
+				regulator-compatible = "LDO5";
+				regulator-min-microvolt = <1800000>;
+				regulator-max-microvolt = <3300000>;
+			};
+		};
+	};
+
+	eeprom@51 {
+		compatible = "atmel,24c32";
+		reg = <0x51>;
+		pagesize = <32>;
+		status = "okay";
+	};
+
+	rv3028: rtc@52 {
+		compatible = "microcrystal,rv3028";
+		reg = <0x52>;
+		trickle-resistor-ohms = <1000>;
+		enable-level-switching-mode;
+		clocks = <&rtcclkout>;
+		status = "okay";
+	};
+};
+
+/* eMMC */
+&usdhc3 {
+	pinctrl-names = "default", "state_100mhz", "state_200mhz";
+	pinctrl-0 = <&pinctrl_usdhc3>;
+	pinctrl-1 = <&pinctrl_usdhc3_100mhz>;
+	pinctrl-2 = <&pinctrl_usdhc3_200mhz>;
+	bus-width = <8>;
+	non-removable;
+	status = "okay";
+};
+
+&wdog1 {
+	pinctrl-names = "default";
+	pinctrl-0 = <&pinctrl_wdog>;
+	fsl,ext-reset-output;
+	status = "okay";
+};
+
+&iomuxc {
+	pinctrl_fec: fecgrp {
+		fsl,pins = <
+			MX8MP_IOMUXC_SAI1_RXD2__ENET1_MDC		0x3
+			MX8MP_IOMUXC_SAI1_RXD3__ENET1_MDIO		0x3
+			MX8MP_IOMUXC_SAI1_RXD4__ENET1_RGMII_RD0		0x91
+			MX8MP_IOMUXC_SAI1_RXD5__ENET1_RGMII_RD1		0x91
+			MX8MP_IOMUXC_SAI1_RXD6__ENET1_RGMII_RD2		0x91
+			MX8MP_IOMUXC_SAI1_RXD7__ENET1_RGMII_RD3		0x91
+			MX8MP_IOMUXC_SAI1_TXC__ENET1_RGMII_RXC		0x91
+			MX8MP_IOMUXC_SAI1_TXFS__ENET1_RGMII_RX_CTL	0x91
+			MX8MP_IOMUXC_SAI1_TXD0__ENET1_RGMII_TD0		0x1f
+			MX8MP_IOMUXC_SAI1_TXD1__ENET1_RGMII_TD1		0x1f
+			MX8MP_IOMUXC_SAI1_TXD2__ENET1_RGMII_TD2		0x1f
+			MX8MP_IOMUXC_SAI1_TXD3__ENET1_RGMII_TD3		0x1f
+			MX8MP_IOMUXC_SAI1_TXD4__ENET1_RGMII_TX_CTL	0x1f
+			MX8MP_IOMUXC_SAI1_TXD5__ENET1_RGMII_TXC		0x1f
+			MX8MP_IOMUXC_GPIO1_IO15__GPIO1_IO15		0x11
+		>;
+	};
+
+	pinctrl_i2c1: i2c1grp {
+		fsl,pins = <
+			MX8MP_IOMUXC_I2C1_SCL__I2C1_SCL		0x400001c3
+			MX8MP_IOMUXC_I2C1_SDA__I2C1_SDA		0x400001c3
+		>;
+	};
+
+	pinctrl_i2c1_gpio: i2c1gpiogrp {
+		fsl,pins = <
+			MX8MP_IOMUXC_I2C1_SCL__GPIO5_IO14	0x1e3
+			MX8MP_IOMUXC_I2C1_SDA__GPIO5_IO15	0x1e3
+		>;
+	};
+
+
+	pinctrl_pmic: pmicirqgrp {
+		fsl,pins = <
+			MX8MP_IOMUXC_SAI1_TXD6__GPIO4_IO18	0x141
+		>;
+	};
+
+	pinctrl_usdhc3: usdhc3grp {
+		fsl,pins = <
+			MX8MP_IOMUXC_NAND_WE_B__USDHC3_CLK	0x190
+			MX8MP_IOMUXC_NAND_WP_B__USDHC3_CMD	0x1d0
+			MX8MP_IOMUXC_NAND_DATA04__USDHC3_DATA0	0x1d0
+			MX8MP_IOMUXC_NAND_DATA05__USDHC3_DATA1	0x1d0
+			MX8MP_IOMUXC_NAND_DATA06__USDHC3_DATA2	0x1d0
+			MX8MP_IOMUXC_NAND_DATA07__USDHC3_DATA3	0x1d0
+			MX8MP_IOMUXC_NAND_RE_B__USDHC3_DATA4	0x1d0
+			MX8MP_IOMUXC_NAND_CE2_B__USDHC3_DATA5	0x1d0
+			MX8MP_IOMUXC_NAND_CE3_B__USDHC3_DATA6	0x1d0
+			MX8MP_IOMUXC_NAND_CLE__USDHC3_DATA7	0x1d0
+			MX8MP_IOMUXC_NAND_CE1_B__USDHC3_STROBE	0x190
+		>;
+	};
+
+	pinctrl_usdhc3_100mhz: usdhc3-100mhzgrp {
+		fsl,pins = <
+			MX8MP_IOMUXC_NAND_WE_B__USDHC3_CLK	0x194
+			MX8MP_IOMUXC_NAND_WP_B__USDHC3_CMD	0x1d4
+			MX8MP_IOMUXC_NAND_DATA04__USDHC3_DATA0	0x1d4
+			MX8MP_IOMUXC_NAND_DATA05__USDHC3_DATA1	0x1d4
+			MX8MP_IOMUXC_NAND_DATA06__USDHC3_DATA2	0x1d4
+			MX8MP_IOMUXC_NAND_DATA07__USDHC3_DATA3	0x1d4
+			MX8MP_IOMUXC_NAND_RE_B__USDHC3_DATA4	0x1d4
+			MX8MP_IOMUXC_NAND_CE2_B__USDHC3_DATA5	0x1d4
+			MX8MP_IOMUXC_NAND_CE3_B__USDHC3_DATA6	0x1d4
+			MX8MP_IOMUXC_NAND_CLE__USDHC3_DATA7	0x1d4
+			MX8MP_IOMUXC_NAND_CE1_B__USDHC3_STROBE	0x194
+		>;
+	};
+
+	pinctrl_usdhc3_200mhz: usdhc3-200mhzgrp {
+		fsl,pins = <
+			MX8MP_IOMUXC_NAND_WE_B__USDHC3_CLK	0x196
+			MX8MP_IOMUXC_NAND_WP_B__USDHC3_CMD	0x1d6
+			MX8MP_IOMUXC_NAND_DATA04__USDHC3_DATA0	0x1d6
+			MX8MP_IOMUXC_NAND_DATA05__USDHC3_DATA1	0x1d6
+			MX8MP_IOMUXC_NAND_DATA06__USDHC3_DATA2	0x1d6
+			MX8MP_IOMUXC_NAND_DATA07__USDHC3_DATA3	0x1d6
+			MX8MP_IOMUXC_NAND_RE_B__USDHC3_DATA4	0x1d6
+			MX8MP_IOMUXC_NAND_CE2_B__USDHC3_DATA5	0x1d6
+			MX8MP_IOMUXC_NAND_CE3_B__USDHC3_DATA6	0x1d6
+			MX8MP_IOMUXC_NAND_CLE__USDHC3_DATA7	0x1d6
+			MX8MP_IOMUXC_NAND_CE1_B__USDHC3_STROBE	0x196
+		>;
+	};
+
+	pinctrl_wdog: wdoggrp {
+		fsl,pins = <
+			MX8MP_IOMUXC_GPIO1_IO02__WDOG1_WDOG_B	0xc6
+		>;
+	};
+};
-- 
2.7.4

[PATCH 4/4] arm64: dts: freescale: Add support for phyBOARD-Pollux-i.MX8MP

[Teresa Remmet]

Add initial support for phyBOARD-Pollux-i.MX8MP.
Supported basic features:
	* eMMC
	* i2c EEPROM
	* i2c RTC
	* i2c LED
	* PMIC
	* debug UART
	* SD card
	* 1Gbit Ethernet (fec)
	* watchdog

Signed-off-by: Teresa Remmet <t.remmet@phytec.de>
---
 arch/arm64/boot/dts/freescale/Makefile             |   1 +
 .../dts/freescale/imx8mp-phyboard-pollux-rdk.dts   |  16 ++
 .../boot/dts/freescale/imx8mp-phyboard-pollux.dtsi | 152 ++++++++++
 .../boot/dts/freescale/imx8mp-phycore-som.dtsi     | 319 +++++++++++++++++++++
 4 files changed, 488 insertions(+)
 create mode 100644 arch/arm64/boot/dts/freescale/imx8mp-phyboard-pollux-rdk.dts
 create mode 100644 arch/arm64/boot/dts/freescale/imx8mp-phyboard-pollux.dtsi
 create mode 100644 arch/arm64/boot/dts/freescale/imx8mp-phycore-som.dtsi

diff --git a/arch/arm64/boot/dts/freescale/Makefile b/arch/arm64/boot/dts/freescale/Makefile
index acfb8af45912..a43b496678be 100644
--- a/arch/arm64/boot/dts/freescale/Makefile
+++ b/arch/arm64/boot/dts/freescale/Makefile
@@ -37,6 +37,7 @@ dtb-$(CONFIG_ARCH_MXC) += imx8mn-evk.dtb
 dtb-$(CONFIG_ARCH_MXC) += imx8mn-ddr4-evk.dtb
 dtb-$(CONFIG_ARCH_MXC) += imx8mn-var-som-symphony.dtb
 dtb-$(CONFIG_ARCH_MXC) += imx8mp-evk.dtb
+dtb-$(CONFIG_ARCH_MXC) += imx8mp-phyboard-pollux-rdk.dtb
 dtb-$(CONFIG_ARCH_MXC) += imx8mq-evk.dtb
 dtb-$(CONFIG_ARCH_MXC) += imx8mq-hummingboard-pulse.dtb
 dtb-$(CONFIG_ARCH_MXC) += imx8mq-librem5-devkit.dtb
diff --git a/arch/arm64/boot/dts/freescale/imx8mp-phyboard-pollux-rdk.dts b/arch/arm64/boot/dts/freescale/imx8mp-phyboard-pollux-rdk.dts
new file mode 100644
index 000000000000..dd64be32c99d
--- /dev/null
+++ b/arch/arm64/boot/dts/freescale/imx8mp-phyboard-pollux-rdk.dts
@@ -0,0 +1,16 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2020 PHYTEC Messtechnik GmbH
+ * Author: Teresa Remmet <t.remmet@phytec.de>
+ */
+
+/dts-v1/;
+
+#include "imx8mp-phycore-som.dtsi"
+#include "imx8mp-phyboard-pollux.dtsi"
+
+/ {
+	model = "PHYTEC phyBOARD-Pollux i.MX8MP";
+	compatible = "phytec,imx8mp-phyboard-pollux-rdk",
+		     "phytec,imx8mp-phycore-som", "fsl,imx8mp";
+};
diff --git a/arch/arm64/boot/dts/freescale/imx8mp-phyboard-pollux.dtsi b/arch/arm64/boot/dts/freescale/imx8mp-phyboard-pollux.dtsi
new file mode 100644
index 000000000000..dbeaa27eb043
--- /dev/null
+++ b/arch/arm64/boot/dts/freescale/imx8mp-phyboard-pollux.dtsi
@@ -0,0 +1,152 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2020 PHYTEC Messtechnik GmbH
+ * Author: Teresa Remmet <t.remmet@phytec.de>
+ */
+
+#include <dt-bindings/leds/leds-pca9532.h>
+#include <dt-bindings/pwm/pwm.h>
+
+/ {
+	chosen {
+		stdout-path = &uart2;
+	};
+
+	reg_usdhc2_vmmc: regulator-usdhc2 {
+		compatible = "regulator-fixed";
+		regulator-name = "VSD_3V3";
+		regulator-min-microvolt = <3300000>;
+		regulator-max-microvolt = <3300000>;
+		gpio = <&gpio2 19 GPIO_ACTIVE_HIGH>;
+		enable-active-high;
+		startup-delay-us = <100>;
+		off-on-delay-us = <12000>;
+	};
+};
+
+&i2c2 {
+	clock-frequency = <400000>;
+	pinctrl-names = "default";
+	pinctrl-0 = <&pinctrl_i2c2>;
+	pinctrl-1 = <&pinctrl_i2c2_gpio>;
+	sda-gpios = <&gpio5 17 (GPIO_ACTIVE_HIGH | GPIO_OPEN_DRAIN)>;
+	scl-gpios = <&gpio5 16 (GPIO_ACTIVE_HIGH | GPIO_OPEN_DRAIN)>;
+	status = "okay";
+
+	eeprom@51 {
+		compatible = "atmel,24c02";
+		reg = <0x51>;
+		pagesize = <16>;
+		status = "okay";
+	};
+
+	leddimmer@62 {
+		compatible = "nxp,pca9533";
+		reg = <0x62>;
+		status = "okay";
+
+		led1 {
+			label = "red:user1";
+			type = <PCA9532_TYPE_LED>;
+		};
+
+		led2 {
+			label = "green:user2";
+			type = <PCA9532_TYPE_LED>;
+		};
+
+		led3 {
+			label = "blue:user3";
+			type = <PCA9532_TYPE_LED>;
+		};
+	};
+};
+
+&snvs_pwrkey {
+	status = "okay";
+};
+
+/* debug console */
+&uart2 {
+	pinctrl-names = "default";
+	pinctrl-0 = <&pinctrl_uart2>;
+	status = "okay";
+};
+
+/* SD-Card */
+&usdhc2 {
+	pinctrl-names = "default", "state_100mhz", "state_200mhz";
+	pinctrl-0 = <&pinctrl_usdhc2>, <&pinctrl_usdhc2_pins>;
+	pinctrl-1 = <&pinctrl_usdhc2_100mhz>, <&pinctrl_usdhc2_pins>;
+	pinctrl-2 = <&pinctrl_usdhc2_200mhz>, <&pinctrl_usdhc2_pins>;
+	cd-gpios = <&gpio2 12 GPIO_ACTIVE_LOW>;
+	vmmc-supply = <&reg_usdhc2_vmmc>;
+	bus-width = <4>;
+	status = "okay";
+};
+
+&iomuxc {
+	pinctrl_i2c2: i2c2grp {
+		fsl,pins = <
+			MX8MP_IOMUXC_I2C2_SCL__I2C2_SCL		0x400001c3
+			MX8MP_IOMUXC_I2C2_SDA__I2C2_SDA		0x400001c3
+		>;
+	};
+
+	pinctrl_i2c2_gpio: i2c2gpiogrp {
+		fsl,pins = <
+			MX8MP_IOMUXC_I2C2_SCL__GPIO5_IO16	0x1e3
+			MX8MP_IOMUXC_I2C2_SDA__GPIO5_IO17	0x1e3
+		>;
+	};
+
+	pinctrl_uart2: uart2grp {
+		fsl,pins = <
+			MX8MP_IOMUXC_UART2_RXD__UART2_DCE_RX	0x49
+			MX8MP_IOMUXC_UART2_TXD__UART2_DCE_TX	0x49
+		>;
+	};
+
+	pinctrl_usdhc2_pins: usdhc2-gpiogrp {
+		fsl,pins = <
+			MX8MP_IOMUXC_SD2_CD_B__GPIO2_IO12	0x1c4
+			MX8MP_IOMUXC_SD2_RESET_B__GPIO2_IO19	0x41
+		>;
+	};
+
+	pinctrl_usdhc2: usdhc2grp {
+		fsl,pins = <
+			MX8MP_IOMUXC_SD2_CLK__USDHC2_CLK	0x190
+			MX8MP_IOMUXC_SD2_CMD__USDHC2_CMD	0x1d0
+			MX8MP_IOMUXC_SD2_DATA0__USDHC2_DATA0	0x1d0
+			MX8MP_IOMUXC_SD2_DATA1__USDHC2_DATA1	0x1d0
+			MX8MP_IOMUXC_SD2_DATA2__USDHC2_DATA2	0x1d0
+			MX8MP_IOMUXC_SD2_DATA3__USDHC2_DATA3	0x1d0
+			MX8MP_IOMUXC_GPIO1_IO04__USDHC2_VSELECT	0xc1
+		>;
+	};
+
+	pinctrl_usdhc2_100mhz: usdhc2-100mhzgrp {
+		fsl,pins = <
+			MX8MP_IOMUXC_SD2_CLK__USDHC2_CLK	0x194
+			MX8MP_IOMUXC_SD2_CMD__USDHC2_CMD	0x1d4
+			MX8MP_IOMUXC_SD2_DATA0__USDHC2_DATA0	0x1d4
+			MX8MP_IOMUXC_SD2_DATA1__USDHC2_DATA1	0x1d4
+			MX8MP_IOMUXC_SD2_DATA2__USDHC2_DATA2	0x1d4
+			MX8MP_IOMUXC_SD2_DATA3__USDHC2_DATA3	0x1d4
+			MX8MP_IOMUXC_GPIO1_IO04__USDHC2_VSELECT	0xc1
+		>;
+	};
+
+	pinctrl_usdhc2_200mhz: usdhc2-200mhzgrp {
+		fsl,pins = <
+			MX8MP_IOMUXC_SD2_CLK__USDHC2_CLK	0x196
+			MX8MP_IOMUXC_SD2_CMD__USDHC2_CMD	0x1d6
+			MX8MP_IOMUXC_SD2_DATA0__USDHC2_DATA0	0x1d6
+			MX8MP_IOMUXC_SD2_DATA1__USDHC2_DATA1	0x1d6
+			MX8MP_IOMUXC_SD2_DATA2__USDHC2_DATA2	0x1d6
+			MX8MP_IOMUXC_SD2_DATA3__USDHC2_DATA3	0x1d6
+			MX8MP_IOMUXC_GPIO1_IO04__USDHC2_VSELECT	0xc1
+		>;
+	};
+};
diff --git a/arch/arm64/boot/dts/freescale/imx8mp-phycore-som.dtsi b/arch/arm64/boot/dts/freescale/imx8mp-phycore-som.dtsi
new file mode 100644
index 000000000000..e1fdfebd8142
--- /dev/null
+++ b/arch/arm64/boot/dts/freescale/imx8mp-phycore-som.dtsi
@@ -0,0 +1,319 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2020 PHYTEC Messtechnik GmbH
+ * Author: Teresa Remmet <t.remmet@phytec.de>
+ */
+
+#include <dt-bindings/net/ti-dp83867.h>
+#include "imx8mp.dtsi"
+
+/ {
+	model = "PHYTEC phyCORE-i.MX8MP";
+	compatible = "phytec,imx8mp-phycore-som", "fsl,imx8mp";
+
+	aliases {
+		rtc0 = &rv3028;
+		rtc1 = &snvs_rtc;
+	};
+
+	memory@40000000 {
+		device_type = "memory";
+		reg = <0x0 0x40000000 0 0x80000000>;
+	};
+
+	rtcclkout: rv3028-clkout {
+		compatible = "fixed-clock";
+		#clock-cells = <0>;
+		clock-frequency = <32768>;
+		clock-output-names = "rv3028-clkout";
+		status = "disabled";
+	};
+};
+
+&A53_0 {
+	cpu-supply = <&buck2>;
+};
+
+&A53_1 {
+	cpu-supply = <&buck2>;
+};
+
+&A53_2 {
+	cpu-supply = <&buck2>;
+};
+
+&A53_3 {
+	cpu-supply = <&buck2>;
+};
+
+/* ethernet 1 */
+&fec {
+	pinctrl-names = "default";
+	pinctrl-0 = <&pinctrl_fec>;
+	phy-mode = "rgmii-id";
+	phy-handle = <&ethphy1>;
+	fsl,magic-packet;
+	status = "okay";
+
+	mdio {
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		ethphy1: ethernet-phy@0 {
+			compatible = "ethernet-phy-ieee802.3-c22";
+			reg = <0>;
+			interrupt-parent = <&gpio1>;
+			interrupts = <15 IRQ_TYPE_EDGE_FALLING>;
+			ti,rx-internal-delay = <DP83867_RGMIIDCTL_2_00_NS>;
+			ti,tx-internal-delay = <DP83867_RGMIIDCTL_2_00_NS>;
+			ti,fifo-depth = <DP83867_PHYCR_FIFO_DEPTH_4_B_NIB>;
+			ti,clk-output-sel = <DP83867_CLK_O_SEL_OFF>;
+			enet-phy-lane-no-swap;
+		};
+	};
+};
+
+&i2c1 {
+	clock-frequency = <400000>;
+	pinctrl-names = "default";
+	pinctrl-0 = <&pinctrl_i2c1>;
+	pinctrl-1 = <&pinctrl_i2c1_gpio>;
+	sda-gpios = <&gpio5 15 (GPIO_ACTIVE_HIGH | GPIO_OPEN_DRAIN)>;
+	scl-gpios = <&gpio5 14 (GPIO_ACTIVE_HIGH | GPIO_OPEN_DRAIN)>;
+	status = "okay";
+
+	pmic: pca9450@25 {
+		reg = <0x25>;
+		compatible = "nxp,pca9450c";
+		pinctrl-names = "default";
+		pinctrl-0 = <&pinctrl_pmic>;
+		interrupt-parent = <&gpio4>;
+		interrupts = <18 IRQ_TYPE_LEVEL_LOW>;
+
+		regulators {
+			#address-cells = <1>;
+			#size-cells = <0>;
+
+			buck1: BUCK1 {
+				reg = <0>;
+				regulator-compatible = "BUCK1";
+				regulator-min-microvolt = <600000>;
+				regulator-max-microvolt = <2187500>;
+				regulator-boot-on;
+				regulator-always-on;
+				regulator-ramp-delay = <3125>;
+			};
+
+			buck2: BUCK2 {
+				reg = <1>;
+				regulator-compatible = "BUCK2";
+				regulator-min-microvolt = <600000>;
+				regulator-max-microvolt = <2187500>;
+				regulator-boot-on;
+				regulator-always-on;
+				regulator-ramp-delay = <3125>;
+			};
+
+			buck4: BUCK4 {
+				reg = <3>;
+				regulator-compatible = "BUCK4";
+				regulator-min-microvolt = <600000>;
+				regulator-max-microvolt = <3400000>;
+				regulator-boot-on;
+				regulator-always-on;
+			};
+
+			buck5: BUCK5 {
+				reg = <4>;
+				regulator-compatible = "BUCK5";
+				regulator-min-microvolt = <600000>;
+				regulator-max-microvolt = <3400000>;
+				regulator-boot-on;
+				regulator-always-on;
+			};
+
+			buck6: BUCK6 {
+				reg = <5>;
+				regulator-compatible = "BUCK6";
+				regulator-min-microvolt = <600000>;
+				regulator-max-microvolt = <3400000>;
+				regulator-boot-on;
+				regulator-always-on;
+			};
+
+			ldo1: LDO1 {
+				reg = <6>;
+				regulator-compatible = "LDO1";
+				regulator-min-microvolt = <1600000>;
+				regulator-max-microvolt = <3300000>;
+				regulator-boot-on;
+				regulator-always-on;
+			};
+
+			ldo2: LDO2 {
+				reg = <7>;
+				regulator-compatible = "LDO2";
+				regulator-min-microvolt = <800000>;
+				regulator-max-microvolt = <1150000>;
+				regulator-boot-on;
+				regulator-always-on;
+			};
+
+			ldo3: LDO3 {
+				reg = <8>;
+				regulator-compatible = "LDO3";
+				regulator-min-microvolt = <800000>;
+				regulator-max-microvolt = <3300000>;
+				regulator-boot-on;
+				regulator-always-on;
+			};
+
+			ldo4: LDO4 {
+				reg = <9>;
+				regulator-compatible = "LDO4";
+				regulator-min-microvolt = <800000>;
+				regulator-max-microvolt = <3300000>;
+				regulator-boot-on;
+				regulator-always-on;
+			};
+
+			ldo5: LDO5 {
+				reg = <10>;
+				regulator-compatible = "LDO5";
+				regulator-min-microvolt = <1800000>;
+				regulator-max-microvolt = <3300000>;
+			};
+		};
+	};
+
+	eeprom@51 {
+		compatible = "atmel,24c32";
+		reg = <0x51>;
+		pagesize = <32>;
+		status = "okay";
+	};
+
+	rv3028: rtc@52 {
+		compatible = "microcrystal,rv3028";
+		reg = <0x52>;
+		trickle-resistor-ohms = <1000>;
+		enable-level-switching-mode;
+		clocks = <&rtcclkout>;
+		status = "okay";
+	};
+};
+
+/* eMMC */
+&usdhc3 {
+	pinctrl-names = "default", "state_100mhz", "state_200mhz";
+	pinctrl-0 = <&pinctrl_usdhc3>;
+	pinctrl-1 = <&pinctrl_usdhc3_100mhz>;
+	pinctrl-2 = <&pinctrl_usdhc3_200mhz>;
+	bus-width = <8>;
+	non-removable;
+	status = "okay";
+};
+
+&wdog1 {
+	pinctrl-names = "default";
+	pinctrl-0 = <&pinctrl_wdog>;
+	fsl,ext-reset-output;
+	status = "okay";
+};
+
+&iomuxc {
+	pinctrl_fec: fecgrp {
+		fsl,pins = <
+			MX8MP_IOMUXC_SAI1_RXD2__ENET1_MDC		0x3
+			MX8MP_IOMUXC_SAI1_RXD3__ENET1_MDIO		0x3
+			MX8MP_IOMUXC_SAI1_RXD4__ENET1_RGMII_RD0		0x91
+			MX8MP_IOMUXC_SAI1_RXD5__ENET1_RGMII_RD1		0x91
+			MX8MP_IOMUXC_SAI1_RXD6__ENET1_RGMII_RD2		0x91
+			MX8MP_IOMUXC_SAI1_RXD7__ENET1_RGMII_RD3		0x91
+			MX8MP_IOMUXC_SAI1_TXC__ENET1_RGMII_RXC		0x91
+			MX8MP_IOMUXC_SAI1_TXFS__ENET1_RGMII_RX_CTL	0x91
+			MX8MP_IOMUXC_SAI1_TXD0__ENET1_RGMII_TD0		0x1f
+			MX8MP_IOMUXC_SAI1_TXD1__ENET1_RGMII_TD1		0x1f
+			MX8MP_IOMUXC_SAI1_TXD2__ENET1_RGMII_TD2		0x1f
+			MX8MP_IOMUXC_SAI1_TXD3__ENET1_RGMII_TD3		0x1f
+			MX8MP_IOMUXC_SAI1_TXD4__ENET1_RGMII_TX_CTL	0x1f
+			MX8MP_IOMUXC_SAI1_TXD5__ENET1_RGMII_TXC		0x1f
+			MX8MP_IOMUXC_GPIO1_IO15__GPIO1_IO15		0x11
+		>;
+	};
+
+	pinctrl_i2c1: i2c1grp {
+		fsl,pins = <
+			MX8MP_IOMUXC_I2C1_SCL__I2C1_SCL		0x400001c3
+			MX8MP_IOMUXC_I2C1_SDA__I2C1_SDA		0x400001c3
+		>;
+	};
+
+	pinctrl_i2c1_gpio: i2c1gpiogrp {
+		fsl,pins = <
+			MX8MP_IOMUXC_I2C1_SCL__GPIO5_IO14	0x1e3
+			MX8MP_IOMUXC_I2C1_SDA__GPIO5_IO15	0x1e3
+		>;
+	};
+
+
+	pinctrl_pmic: pmicirqgrp {
+		fsl,pins = <
+			MX8MP_IOMUXC_SAI1_TXD6__GPIO4_IO18	0x141
+		>;
+	};
+
+	pinctrl_usdhc3: usdhc3grp {
+		fsl,pins = <
+			MX8MP_IOMUXC_NAND_WE_B__USDHC3_CLK	0x190
+			MX8MP_IOMUXC_NAND_WP_B__USDHC3_CMD	0x1d0
+			MX8MP_IOMUXC_NAND_DATA04__USDHC3_DATA0	0x1d0
+			MX8MP_IOMUXC_NAND_DATA05__USDHC3_DATA1	0x1d0
+			MX8MP_IOMUXC_NAND_DATA06__USDHC3_DATA2	0x1d0
+			MX8MP_IOMUXC_NAND_DATA07__USDHC3_DATA3	0x1d0
+			MX8MP_IOMUXC_NAND_RE_B__USDHC3_DATA4	0x1d0
+			MX8MP_IOMUXC_NAND_CE2_B__USDHC3_DATA5	0x1d0
+			MX8MP_IOMUXC_NAND_CE3_B__USDHC3_DATA6	0x1d0
+			MX8MP_IOMUXC_NAND_CLE__USDHC3_DATA7	0x1d0
+			MX8MP_IOMUXC_NAND_CE1_B__USDHC3_STROBE	0x190
+		>;
+	};
+
+	pinctrl_usdhc3_100mhz: usdhc3-100mhzgrp {
+		fsl,pins = <
+			MX8MP_IOMUXC_NAND_WE_B__USDHC3_CLK	0x194
+			MX8MP_IOMUXC_NAND_WP_B__USDHC3_CMD	0x1d4
+			MX8MP_IOMUXC_NAND_DATA04__USDHC3_DATA0	0x1d4
+			MX8MP_IOMUXC_NAND_DATA05__USDHC3_DATA1	0x1d4
+			MX8MP_IOMUXC_NAND_DATA06__USDHC3_DATA2	0x1d4
+			MX8MP_IOMUXC_NAND_DATA07__USDHC3_DATA3	0x1d4
+			MX8MP_IOMUXC_NAND_RE_B__USDHC3_DATA4	0x1d4
+			MX8MP_IOMUXC_NAND_CE2_B__USDHC3_DATA5	0x1d4
+			MX8MP_IOMUXC_NAND_CE3_B__USDHC3_DATA6	0x1d4
+			MX8MP_IOMUXC_NAND_CLE__USDHC3_DATA7	0x1d4
+			MX8MP_IOMUXC_NAND_CE1_B__USDHC3_STROBE	0x194
+		>;
+	};
+
+	pinctrl_usdhc3_200mhz: usdhc3-200mhzgrp {
+		fsl,pins = <
+			MX8MP_IOMUXC_NAND_WE_B__USDHC3_CLK	0x196
+			MX8MP_IOMUXC_NAND_WP_B__USDHC3_CMD	0x1d6
+			MX8MP_IOMUXC_NAND_DATA04__USDHC3_DATA0	0x1d6
+			MX8MP_IOMUXC_NAND_DATA05__USDHC3_DATA1	0x1d6
+			MX8MP_IOMUXC_NAND_DATA06__USDHC3_DATA2	0x1d6
+			MX8MP_IOMUXC_NAND_DATA07__USDHC3_DATA3	0x1d6
+			MX8MP_IOMUXC_NAND_RE_B__USDHC3_DATA4	0x1d6
+			MX8MP_IOMUXC_NAND_CE2_B__USDHC3_DATA5	0x1d6
+			MX8MP_IOMUXC_NAND_CE3_B__USDHC3_DATA6	0x1d6
+			MX8MP_IOMUXC_NAND_CLE__USDHC3_DATA7	0x1d6
+			MX8MP_IOMUXC_NAND_CE1_B__USDHC3_STROBE	0x196
+		>;
+	};
+
+	pinctrl_wdog: wdoggrp {
+		fsl,pins = <
+			MX8MP_IOMUXC_GPIO1_IO02__WDOG1_WDOG_B	0xc6
+		>;
+	};
+};
-- 
2.7.4


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 4/4] arm64: dts: freescale: Add support for phyBOARD-Pollux-i.MX8MP

[Krzysztof Kozlowski]

On Fri, Dec 04, 2020 at 09:33:02PM +0100, Teresa Remmet wrote:
> Add initial support for phyBOARD-Pollux-i.MX8MP.
> Supported basic features:
> 	* eMMC
> 	* i2c EEPROM
> 	* i2c RTC
> 	* i2c LED
> 	* PMIC
> 	* debug UART
> 	* SD card
> 	* 1Gbit Ethernet (fec)
> 	* watchdog
> 
> Signed-off-by: Teresa Remmet <t.remmet@phytec.de>
> ---
>  arch/arm64/boot/dts/freescale/Makefile             |   1 +
>  .../dts/freescale/imx8mp-phyboard-pollux-rdk.dts   |  16 ++
>  .../boot/dts/freescale/imx8mp-phyboard-pollux.dtsi | 152 ++++++++++
>  .../boot/dts/freescale/imx8mp-phycore-som.dtsi     | 319 +++++++++++++++++++++
>  4 files changed, 488 insertions(+)
>  create mode 100644 arch/arm64/boot/dts/freescale/imx8mp-phyboard-pollux-rdk.dts
>  create mode 100644 arch/arm64/boot/dts/freescale/imx8mp-phyboard-pollux.dtsi
>  create mode 100644 arch/arm64/boot/dts/freescale/imx8mp-phycore-som.dtsi
> 
> diff --git a/arch/arm64/boot/dts/freescale/Makefile b/arch/arm64/boot/dts/freescale/Makefile
> index acfb8af45912..a43b496678be 100644
> --- a/arch/arm64/boot/dts/freescale/Makefile
> +++ b/arch/arm64/boot/dts/freescale/Makefile
> @@ -37,6 +37,7 @@ dtb-$(CONFIG_ARCH_MXC) += imx8mn-evk.dtb
>  dtb-$(CONFIG_ARCH_MXC) += imx8mn-ddr4-evk.dtb
>  dtb-$(CONFIG_ARCH_MXC) += imx8mn-var-som-symphony.dtb
>  dtb-$(CONFIG_ARCH_MXC) += imx8mp-evk.dtb
> +dtb-$(CONFIG_ARCH_MXC) += imx8mp-phyboard-pollux-rdk.dtb
>  dtb-$(CONFIG_ARCH_MXC) += imx8mq-evk.dtb
>  dtb-$(CONFIG_ARCH_MXC) += imx8mq-hummingboard-pulse.dtb
>  dtb-$(CONFIG_ARCH_MXC) += imx8mq-librem5-devkit.dtb
> diff --git a/arch/arm64/boot/dts/freescale/imx8mp-phyboard-pollux-rdk.dts b/arch/arm64/boot/dts/freescale/imx8mp-phyboard-pollux-rdk.dts
> new file mode 100644
> index 000000000000..dd64be32c99d
> --- /dev/null
> +++ b/arch/arm64/boot/dts/freescale/imx8mp-phyboard-pollux-rdk.dts
> @@ -0,0 +1,16 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (C) 2020 PHYTEC Messtechnik GmbH
> + * Author: Teresa Remmet <t.remmet@phytec.de>
> + */
> +
> +/dts-v1/;
> +
> +#include "imx8mp-phycore-som.dtsi"
> +#include "imx8mp-phyboard-pollux.dtsi"
> +
> +/ {
> +	model = "PHYTEC phyBOARD-Pollux i.MX8MP";
> +	compatible = "phytec,imx8mp-phyboard-pollux-rdk",
> +		     "phytec,imx8mp-phycore-som", "fsl,imx8mp";

This is the purpose of this file? Why having a DTS to include DTSI only?
Usually there is just DTSI for SOM and DTS fot the board.

> +};
> diff --git a/arch/arm64/boot/dts/freescale/imx8mp-phyboard-pollux.dtsi b/arch/arm64/boot/dts/freescale/imx8mp-phyboard-pollux.dtsi
> new file mode 100644
> index 000000000000..dbeaa27eb043
> --- /dev/null
> +++ b/arch/arm64/boot/dts/freescale/imx8mp-phyboard-pollux.dtsi
> @@ -0,0 +1,152 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (C) 2020 PHYTEC Messtechnik GmbH
> + * Author: Teresa Remmet <t.remmet@phytec.de>
> + */
> +
> +#include <dt-bindings/leds/leds-pca9532.h>
> +#include <dt-bindings/pwm/pwm.h>
> +
> +/ {
> +	chosen {
> +		stdout-path = &uart2;
> +	};
> +
> +	reg_usdhc2_vmmc: regulator-usdhc2 {
> +		compatible = "regulator-fixed";
> +		regulator-name = "VSD_3V3";
> +		regulator-min-microvolt = <3300000>;
> +		regulator-max-microvolt = <3300000>;
> +		gpio = <&gpio2 19 GPIO_ACTIVE_HIGH>;
> +		enable-active-high;
> +		startup-delay-us = <100>;
> +		off-on-delay-us = <12000>;
> +	};
> +};
> +
> +&i2c2 {
> +	clock-frequency = <400000>;
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&pinctrl_i2c2>;
> +	pinctrl-1 = <&pinctrl_i2c2_gpio>;
> +	sda-gpios = <&gpio5 17 (GPIO_ACTIVE_HIGH | GPIO_OPEN_DRAIN)>;
> +	scl-gpios = <&gpio5 16 (GPIO_ACTIVE_HIGH | GPIO_OPEN_DRAIN)>;
> +	status = "okay";
> +
> +	eeprom@51 {
> +		compatible = "atmel,24c02";
> +		reg = <0x51>;
> +		pagesize = <16>;
> +		status = "okay";
> +	};
> +
> +	leddimmer@62 {

I think name "leds" is more appropriate.

> +		compatible = "nxp,pca9533";
> +		reg = <0x62>;
> +		status = "okay";
> +
> +		led1 {
> +			label = "red:user1";
> +			type = <PCA9532_TYPE_LED>;
> +		};
> +
> +		led2 {
> +			label = "green:user2";
> +			type = <PCA9532_TYPE_LED>;
> +		};
> +
> +		led3 {
> +			label = "blue:user3";
> +			type = <PCA9532_TYPE_LED>;
> +		};
> +	};
> +};
> +
> +&snvs_pwrkey {
> +	status = "okay";
> +};
> +
> +/* debug console */
> +&uart2 {
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&pinctrl_uart2>;
> +	status = "okay";
> +};
> +
> +/* SD-Card */
> +&usdhc2 {
> +	pinctrl-names = "default", "state_100mhz", "state_200mhz";
> +	pinctrl-0 = <&pinctrl_usdhc2>, <&pinctrl_usdhc2_pins>;
> +	pinctrl-1 = <&pinctrl_usdhc2_100mhz>, <&pinctrl_usdhc2_pins>;
> +	pinctrl-2 = <&pinctrl_usdhc2_200mhz>, <&pinctrl_usdhc2_pins>;
> +	cd-gpios = <&gpio2 12 GPIO_ACTIVE_LOW>;
> +	vmmc-supply = <&reg_usdhc2_vmmc>;
> +	bus-width = <4>;
> +	status = "okay";
> +};
> +
> +&iomuxc {
> +	pinctrl_i2c2: i2c2grp {
> +		fsl,pins = <
> +			MX8MP_IOMUXC_I2C2_SCL__I2C2_SCL		0x400001c3
> +			MX8MP_IOMUXC_I2C2_SDA__I2C2_SDA		0x400001c3
> +		>;
> +	};
> +
> +	pinctrl_i2c2_gpio: i2c2gpiogrp {
> +		fsl,pins = <
> +			MX8MP_IOMUXC_I2C2_SCL__GPIO5_IO16	0x1e3
> +			MX8MP_IOMUXC_I2C2_SDA__GPIO5_IO17	0x1e3
> +		>;
> +	};
> +
> +	pinctrl_uart2: uart2grp {
> +		fsl,pins = <
> +			MX8MP_IOMUXC_UART2_RXD__UART2_DCE_RX	0x49
> +			MX8MP_IOMUXC_UART2_TXD__UART2_DCE_TX	0x49
> +		>;
> +	};
> +
> +	pinctrl_usdhc2_pins: usdhc2-gpiogrp {
> +		fsl,pins = <
> +			MX8MP_IOMUXC_SD2_CD_B__GPIO2_IO12	0x1c4
> +			MX8MP_IOMUXC_SD2_RESET_B__GPIO2_IO19	0x41
> +		>;
> +	};
> +
> +	pinctrl_usdhc2: usdhc2grp {
> +		fsl,pins = <
> +			MX8MP_IOMUXC_SD2_CLK__USDHC2_CLK	0x190
> +			MX8MP_IOMUXC_SD2_CMD__USDHC2_CMD	0x1d0
> +			MX8MP_IOMUXC_SD2_DATA0__USDHC2_DATA0	0x1d0
> +			MX8MP_IOMUXC_SD2_DATA1__USDHC2_DATA1	0x1d0
> +			MX8MP_IOMUXC_SD2_DATA2__USDHC2_DATA2	0x1d0
> +			MX8MP_IOMUXC_SD2_DATA3__USDHC2_DATA3	0x1d0
> +			MX8MP_IOMUXC_GPIO1_IO04__USDHC2_VSELECT	0xc1
> +		>;
> +	};
> +
> +	pinctrl_usdhc2_100mhz: usdhc2-100mhzgrp {
> +		fsl,pins = <
> +			MX8MP_IOMUXC_SD2_CLK__USDHC2_CLK	0x194
> +			MX8MP_IOMUXC_SD2_CMD__USDHC2_CMD	0x1d4
> +			MX8MP_IOMUXC_SD2_DATA0__USDHC2_DATA0	0x1d4
> +			MX8MP_IOMUXC_SD2_DATA1__USDHC2_DATA1	0x1d4
> +			MX8MP_IOMUXC_SD2_DATA2__USDHC2_DATA2	0x1d4
> +			MX8MP_IOMUXC_SD2_DATA3__USDHC2_DATA3	0x1d4
> +			MX8MP_IOMUXC_GPIO1_IO04__USDHC2_VSELECT	0xc1
> +		>;
> +	};
> +
> +	pinctrl_usdhc2_200mhz: usdhc2-200mhzgrp {
> +		fsl,pins = <
> +			MX8MP_IOMUXC_SD2_CLK__USDHC2_CLK	0x196
> +			MX8MP_IOMUXC_SD2_CMD__USDHC2_CMD	0x1d6
> +			MX8MP_IOMUXC_SD2_DATA0__USDHC2_DATA0	0x1d6
> +			MX8MP_IOMUXC_SD2_DATA1__USDHC2_DATA1	0x1d6
> +			MX8MP_IOMUXC_SD2_DATA2__USDHC2_DATA2	0x1d6
> +			MX8MP_IOMUXC_SD2_DATA3__USDHC2_DATA3	0x1d6
> +			MX8MP_IOMUXC_GPIO1_IO04__USDHC2_VSELECT	0xc1
> +		>;
> +	};
> +};
> diff --git a/arch/arm64/boot/dts/freescale/imx8mp-phycore-som.dtsi b/arch/arm64/boot/dts/freescale/imx8mp-phycore-som.dtsi
> new file mode 100644
> index 000000000000..e1fdfebd8142
> --- /dev/null
> +++ b/arch/arm64/boot/dts/freescale/imx8mp-phycore-som.dtsi
> @@ -0,0 +1,319 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (C) 2020 PHYTEC Messtechnik GmbH
> + * Author: Teresa Remmet <t.remmet@phytec.de>
> + */
> +
> +#include <dt-bindings/net/ti-dp83867.h>
> +#include "imx8mp.dtsi"
> +
> +/ {
> +	model = "PHYTEC phyCORE-i.MX8MP";
> +	compatible = "phytec,imx8mp-phycore-som", "fsl,imx8mp";
> +
> +	aliases {
> +		rtc0 = &rv3028;
> +		rtc1 = &snvs_rtc;
> +	};
> +
> +	memory@40000000 {
> +		device_type = "memory";
> +		reg = <0x0 0x40000000 0 0x80000000>;
> +	};
> +
> +	rtcclkout: rv3028-clkout {

Is it really a separate oscillator giving 32 kHz? Or maybe this is
actually part of PMIC?

In any case node name should reflect class of a device so just "clock".

> +		compatible = "fixed-clock";
> +		#clock-cells = <0>;
> +		clock-frequency = <32768>;
> +		clock-output-names = "rv3028-clkout";
> +		status = "disabled";
> +	};
> +};
> +
> +&A53_0 {
> +	cpu-supply = <&buck2>;
> +};
> +
> +&A53_1 {
> +	cpu-supply = <&buck2>;
> +};
> +
> +&A53_2 {
> +	cpu-supply = <&buck2>;
> +};
> +
> +&A53_3 {
> +	cpu-supply = <&buck2>;
> +};
> +
> +/* ethernet 1 */
> +&fec {
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&pinctrl_fec>;
> +	phy-mode = "rgmii-id";
> +	phy-handle = <&ethphy1>;
> +	fsl,magic-packet;
> +	status = "okay";
> +
> +	mdio {
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +
> +		ethphy1: ethernet-phy@0 {
> +			compatible = "ethernet-phy-ieee802.3-c22";
> +			reg = <0>;
> +			interrupt-parent = <&gpio1>;
> +			interrupts = <15 IRQ_TYPE_EDGE_FALLING>;
> +			ti,rx-internal-delay = <DP83867_RGMIIDCTL_2_00_NS>;
> +			ti,tx-internal-delay = <DP83867_RGMIIDCTL_2_00_NS>;
> +			ti,fifo-depth = <DP83867_PHYCR_FIFO_DEPTH_4_B_NIB>;
> +			ti,clk-output-sel = <DP83867_CLK_O_SEL_OFF>;
> +			enet-phy-lane-no-swap;
> +		};
> +	};
> +};
> +
> +&i2c1 {
> +	clock-frequency = <400000>;
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&pinctrl_i2c1>;
> +	pinctrl-1 = <&pinctrl_i2c1_gpio>;
> +	sda-gpios = <&gpio5 15 (GPIO_ACTIVE_HIGH | GPIO_OPEN_DRAIN)>;
> +	scl-gpios = <&gpio5 14 (GPIO_ACTIVE_HIGH | GPIO_OPEN_DRAIN)>;
> +	status = "okay";
> +
> +	pmic: pca9450@25 {

Node name: pmic.

> +		reg = <0x25>;
> +		compatible = "nxp,pca9450c";
> +		pinctrl-names = "default";
> +		pinctrl-0 = <&pinctrl_pmic>;
> +		interrupt-parent = <&gpio4>;
> +		interrupts = <18 IRQ_TYPE_LEVEL_LOW>;
> +
> +		regulators {
> +			#address-cells = <1>;
> +			#size-cells = <0>;
> +
> +			buck1: BUCK1 {
> +				reg = <0>;

Does this all pass dtc W=1 builds and dtschema? The node name does not
have unit address. Bindings explicitly do not allow it... This looks
wrong.

> +				regulator-compatible = "BUCK1";
> +				regulator-min-microvolt = <600000>;
> +				regulator-max-microvolt = <2187500>;
> +				regulator-boot-on;
> +				regulator-always-on;
> +				regulator-ramp-delay = <3125>;
> +			};
> +
> +			buck2: BUCK2 {
> +				reg = <1>;
> +				regulator-compatible = "BUCK2";
> +				regulator-min-microvolt = <600000>;
> +				regulator-max-microvolt = <2187500>;
> +				regulator-boot-on;
> +				regulator-always-on;
> +				regulator-ramp-delay = <3125>;
> +			};
> +
> +			buck4: BUCK4 {
> +				reg = <3>;
> +				regulator-compatible = "BUCK4";
> +				regulator-min-microvolt = <600000>;
> +				regulator-max-microvolt = <3400000>;
> +				regulator-boot-on;
> +				regulator-always-on;
> +			};
> +
> +			buck5: BUCK5 {
> +				reg = <4>;
> +				regulator-compatible = "BUCK5";
> +				regulator-min-microvolt = <600000>;
> +				regulator-max-microvolt = <3400000>;
> +				regulator-boot-on;
> +				regulator-always-on;
> +			};
> +
> +			buck6: BUCK6 {
> +				reg = <5>;
> +				regulator-compatible = "BUCK6";
> +				regulator-min-microvolt = <600000>;
> +				regulator-max-microvolt = <3400000>;
> +				regulator-boot-on;
> +				regulator-always-on;
> +			};
> +
> +			ldo1: LDO1 {
> +				reg = <6>;
> +				regulator-compatible = "LDO1";
> +				regulator-min-microvolt = <1600000>;
> +				regulator-max-microvolt = <3300000>;
> +				regulator-boot-on;
> +				regulator-always-on;
> +			};
> +
> +			ldo2: LDO2 {
> +				reg = <7>;
> +				regulator-compatible = "LDO2";
> +				regulator-min-microvolt = <800000>;
> +				regulator-max-microvolt = <1150000>;
> +				regulator-boot-on;
> +				regulator-always-on;
> +			};
> +
> +			ldo3: LDO3 {
> +				reg = <8>;
> +				regulator-compatible = "LDO3";
> +				regulator-min-microvolt = <800000>;
> +				regulator-max-microvolt = <3300000>;
> +				regulator-boot-on;
> +				regulator-always-on;
> +			};
> +
> +			ldo4: LDO4 {
> +				reg = <9>;
> +				regulator-compatible = "LDO4";
> +				regulator-min-microvolt = <800000>;
> +				regulator-max-microvolt = <3300000>;
> +				regulator-boot-on;
> +				regulator-always-on;
> +			};
> +
> +			ldo5: LDO5 {
> +				reg = <10>;
> +				regulator-compatible = "LDO5";
> +				regulator-min-microvolt = <1800000>;
> +				regulator-max-microvolt = <3300000>;
> +			};
> +		};
> +	};
> +
> +	eeprom@51 {
> +		compatible = "atmel,24c32";
> +		reg = <0x51>;
> +		pagesize = <32>;
> +		status = "okay";
> +	};
> +
> +	rv3028: rtc@52 {
> +		compatible = "microcrystal,rv3028";
> +		reg = <0x52>;
> +		trickle-resistor-ohms = <1000>;
> +		enable-level-switching-mode;
> +		clocks = <&rtcclkout>;
> +		status = "okay";
> +	};
> +};
> +
> +/* eMMC */
> +&usdhc3 {
> +	pinctrl-names = "default", "state_100mhz", "state_200mhz";
> +	pinctrl-0 = <&pinctrl_usdhc3>;
> +	pinctrl-1 = <&pinctrl_usdhc3_100mhz>;
> +	pinctrl-2 = <&pinctrl_usdhc3_200mhz>;
> +	bus-width = <8>;
> +	non-removable;
> +	status = "okay";
> +};
> +
> +&wdog1 {
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&pinctrl_wdog>;
> +	fsl,ext-reset-output;
> +	status = "okay";
> +};
> +
> +&iomuxc {
> +	pinctrl_fec: fecgrp {
> +		fsl,pins = <
> +			MX8MP_IOMUXC_SAI1_RXD2__ENET1_MDC		0x3
> +			MX8MP_IOMUXC_SAI1_RXD3__ENET1_MDIO		0x3
> +			MX8MP_IOMUXC_SAI1_RXD4__ENET1_RGMII_RD0		0x91
> +			MX8MP_IOMUXC_SAI1_RXD5__ENET1_RGMII_RD1		0x91
> +			MX8MP_IOMUXC_SAI1_RXD6__ENET1_RGMII_RD2		0x91
> +			MX8MP_IOMUXC_SAI1_RXD7__ENET1_RGMII_RD3		0x91
> +			MX8MP_IOMUXC_SAI1_TXC__ENET1_RGMII_RXC		0x91
> +			MX8MP_IOMUXC_SAI1_TXFS__ENET1_RGMII_RX_CTL	0x91
> +			MX8MP_IOMUXC_SAI1_TXD0__ENET1_RGMII_TD0		0x1f
> +			MX8MP_IOMUXC_SAI1_TXD1__ENET1_RGMII_TD1		0x1f
> +			MX8MP_IOMUXC_SAI1_TXD2__ENET1_RGMII_TD2		0x1f
> +			MX8MP_IOMUXC_SAI1_TXD3__ENET1_RGMII_TD3		0x1f
> +			MX8MP_IOMUXC_SAI1_TXD4__ENET1_RGMII_TX_CTL	0x1f
> +			MX8MP_IOMUXC_SAI1_TXD5__ENET1_RGMII_TXC		0x1f
> +			MX8MP_IOMUXC_GPIO1_IO15__GPIO1_IO15		0x11
> +		>;
> +	};
> +
> +	pinctrl_i2c1: i2c1grp {
> +		fsl,pins = <
> +			MX8MP_IOMUXC_I2C1_SCL__I2C1_SCL		0x400001c3
> +			MX8MP_IOMUXC_I2C1_SDA__I2C1_SDA		0x400001c3
> +		>;
> +	};
> +
> +	pinctrl_i2c1_gpio: i2c1gpiogrp {
> +		fsl,pins = <
> +			MX8MP_IOMUXC_I2C1_SCL__GPIO5_IO14	0x1e3
> +			MX8MP_IOMUXC_I2C1_SDA__GPIO5_IO15	0x1e3
> +		>;
> +	};
> +
> +

Too many line breaks.

Best regards,
Krzysztof

Re: [PATCH 4/4] arm64: dts: freescale: Add support for phyBOARD-Pollux-i.MX8MP

[Krzysztof Kozlowski]

On Fri, Dec 04, 2020 at 09:33:02PM +0100, Teresa Remmet wrote:
> Add initial support for phyBOARD-Pollux-i.MX8MP.
> Supported basic features:
> 	* eMMC
> 	* i2c EEPROM
> 	* i2c RTC
> 	* i2c LED
> 	* PMIC
> 	* debug UART
> 	* SD card
> 	* 1Gbit Ethernet (fec)
> 	* watchdog
> 
> Signed-off-by: Teresa Remmet <t.remmet@phytec.de>
> ---
>  arch/arm64/boot/dts/freescale/Makefile             |   1 +
>  .../dts/freescale/imx8mp-phyboard-pollux-rdk.dts   |  16 ++
>  .../boot/dts/freescale/imx8mp-phyboard-pollux.dtsi | 152 ++++++++++
>  .../boot/dts/freescale/imx8mp-phycore-som.dtsi     | 319 +++++++++++++++++++++
>  4 files changed, 488 insertions(+)
>  create mode 100644 arch/arm64/boot/dts/freescale/imx8mp-phyboard-pollux-rdk.dts
>  create mode 100644 arch/arm64/boot/dts/freescale/imx8mp-phyboard-pollux.dtsi
>  create mode 100644 arch/arm64/boot/dts/freescale/imx8mp-phycore-som.dtsi
> 
> diff --git a/arch/arm64/boot/dts/freescale/Makefile b/arch/arm64/boot/dts/freescale/Makefile
> index acfb8af45912..a43b496678be 100644
> --- a/arch/arm64/boot/dts/freescale/Makefile
> +++ b/arch/arm64/boot/dts/freescale/Makefile
> @@ -37,6 +37,7 @@ dtb-$(CONFIG_ARCH_MXC) += imx8mn-evk.dtb
>  dtb-$(CONFIG_ARCH_MXC) += imx8mn-ddr4-evk.dtb
>  dtb-$(CONFIG_ARCH_MXC) += imx8mn-var-som-symphony.dtb
>  dtb-$(CONFIG_ARCH_MXC) += imx8mp-evk.dtb
> +dtb-$(CONFIG_ARCH_MXC) += imx8mp-phyboard-pollux-rdk.dtb
>  dtb-$(CONFIG_ARCH_MXC) += imx8mq-evk.dtb
>  dtb-$(CONFIG_ARCH_MXC) += imx8mq-hummingboard-pulse.dtb
>  dtb-$(CONFIG_ARCH_MXC) += imx8mq-librem5-devkit.dtb
> diff --git a/arch/arm64/boot/dts/freescale/imx8mp-phyboard-pollux-rdk.dts b/arch/arm64/boot/dts/freescale/imx8mp-phyboard-pollux-rdk.dts
> new file mode 100644
> index 000000000000..dd64be32c99d
> --- /dev/null
> +++ b/arch/arm64/boot/dts/freescale/imx8mp-phyboard-pollux-rdk.dts
> @@ -0,0 +1,16 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (C) 2020 PHYTEC Messtechnik GmbH
> + * Author: Teresa Remmet <t.remmet@phytec.de>
> + */
> +
> +/dts-v1/;
> +
> +#include "imx8mp-phycore-som.dtsi"
> +#include "imx8mp-phyboard-pollux.dtsi"
> +
> +/ {
> +	model = "PHYTEC phyBOARD-Pollux i.MX8MP";
> +	compatible = "phytec,imx8mp-phyboard-pollux-rdk",
> +		     "phytec,imx8mp-phycore-som", "fsl,imx8mp";

This is the purpose of this file? Why having a DTS to include DTSI only?
Usually there is just DTSI for SOM and DTS fot the board.

> +};
> diff --git a/arch/arm64/boot/dts/freescale/imx8mp-phyboard-pollux.dtsi b/arch/arm64/boot/dts/freescale/imx8mp-phyboard-pollux.dtsi
> new file mode 100644
> index 000000000000..dbeaa27eb043
> --- /dev/null
> +++ b/arch/arm64/boot/dts/freescale/imx8mp-phyboard-pollux.dtsi
> @@ -0,0 +1,152 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (C) 2020 PHYTEC Messtechnik GmbH
> + * Author: Teresa Remmet <t.remmet@phytec.de>
> + */
> +
> +#include <dt-bindings/leds/leds-pca9532.h>
> +#include <dt-bindings/pwm/pwm.h>
> +
> +/ {
> +	chosen {
> +		stdout-path = &uart2;
> +	};
> +
> +	reg_usdhc2_vmmc: regulator-usdhc2 {
> +		compatible = "regulator-fixed";
> +		regulator-name = "VSD_3V3";
> +		regulator-min-microvolt = <3300000>;
> +		regulator-max-microvolt = <3300000>;
> +		gpio = <&gpio2 19 GPIO_ACTIVE_HIGH>;
> +		enable-active-high;
> +		startup-delay-us = <100>;
> +		off-on-delay-us = <12000>;
> +	};
> +};
> +
> +&i2c2 {
> +	clock-frequency = <400000>;
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&pinctrl_i2c2>;
> +	pinctrl-1 = <&pinctrl_i2c2_gpio>;
> +	sda-gpios = <&gpio5 17 (GPIO_ACTIVE_HIGH | GPIO_OPEN_DRAIN)>;
> +	scl-gpios = <&gpio5 16 (GPIO_ACTIVE_HIGH | GPIO_OPEN_DRAIN)>;
> +	status = "okay";
> +
> +	eeprom@51 {
> +		compatible = "atmel,24c02";
> +		reg = <0x51>;
> +		pagesize = <16>;
> +		status = "okay";
> +	};
> +
> +	leddimmer@62 {

I think name "leds" is more appropriate.

> +		compatible = "nxp,pca9533";
> +		reg = <0x62>;
> +		status = "okay";
> +
> +		led1 {
> +			label = "red:user1";
> +			type = <PCA9532_TYPE_LED>;
> +		};
> +
> +		led2 {
> +			label = "green:user2";
> +			type = <PCA9532_TYPE_LED>;
> +		};
> +
> +		led3 {
> +			label = "blue:user3";
> +			type = <PCA9532_TYPE_LED>;
> +		};
> +	};
> +};
> +
> +&snvs_pwrkey {
> +	status = "okay";
> +};
> +
> +/* debug console */
> +&uart2 {
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&pinctrl_uart2>;
> +	status = "okay";
> +};
> +
> +/* SD-Card */
> +&usdhc2 {
> +	pinctrl-names = "default", "state_100mhz", "state_200mhz";
> +	pinctrl-0 = <&pinctrl_usdhc2>, <&pinctrl_usdhc2_pins>;
> +	pinctrl-1 = <&pinctrl_usdhc2_100mhz>, <&pinctrl_usdhc2_pins>;
> +	pinctrl-2 = <&pinctrl_usdhc2_200mhz>, <&pinctrl_usdhc2_pins>;
> +	cd-gpios = <&gpio2 12 GPIO_ACTIVE_LOW>;
> +	vmmc-supply = <&reg_usdhc2_vmmc>;
> +	bus-width = <4>;
> +	status = "okay";
> +};
> +
> +&iomuxc {
> +	pinctrl_i2c2: i2c2grp {
> +		fsl,pins = <
> +			MX8MP_IOMUXC_I2C2_SCL__I2C2_SCL		0x400001c3
> +			MX8MP_IOMUXC_I2C2_SDA__I2C2_SDA		0x400001c3
> +		>;
> +	};
> +
> +	pinctrl_i2c2_gpio: i2c2gpiogrp {
> +		fsl,pins = <
> +			MX8MP_IOMUXC_I2C2_SCL__GPIO5_IO16	0x1e3
> +			MX8MP_IOMUXC_I2C2_SDA__GPIO5_IO17	0x1e3
> +		>;
> +	};
> +
> +	pinctrl_uart2: uart2grp {
> +		fsl,pins = <
> +			MX8MP_IOMUXC_UART2_RXD__UART2_DCE_RX	0x49
> +			MX8MP_IOMUXC_UART2_TXD__UART2_DCE_TX	0x49
> +		>;
> +	};
> +
> +	pinctrl_usdhc2_pins: usdhc2-gpiogrp {
> +		fsl,pins = <
> +			MX8MP_IOMUXC_SD2_CD_B__GPIO2_IO12	0x1c4
> +			MX8MP_IOMUXC_SD2_RESET_B__GPIO2_IO19	0x41
> +		>;
> +	};
> +
> +	pinctrl_usdhc2: usdhc2grp {
> +		fsl,pins = <
> +			MX8MP_IOMUXC_SD2_CLK__USDHC2_CLK	0x190
> +			MX8MP_IOMUXC_SD2_CMD__USDHC2_CMD	0x1d0
> +			MX8MP_IOMUXC_SD2_DATA0__USDHC2_DATA0	0x1d0
> +			MX8MP_IOMUXC_SD2_DATA1__USDHC2_DATA1	0x1d0
> +			MX8MP_IOMUXC_SD2_DATA2__USDHC2_DATA2	0x1d0
> +			MX8MP_IOMUXC_SD2_DATA3__USDHC2_DATA3	0x1d0
> +			MX8MP_IOMUXC_GPIO1_IO04__USDHC2_VSELECT	0xc1
> +		>;
> +	};
> +
> +	pinctrl_usdhc2_100mhz: usdhc2-100mhzgrp {
> +		fsl,pins = <
> +			MX8MP_IOMUXC_SD2_CLK__USDHC2_CLK	0x194
> +			MX8MP_IOMUXC_SD2_CMD__USDHC2_CMD	0x1d4
> +			MX8MP_IOMUXC_SD2_DATA0__USDHC2_DATA0	0x1d4
> +			MX8MP_IOMUXC_SD2_DATA1__USDHC2_DATA1	0x1d4
> +			MX8MP_IOMUXC_SD2_DATA2__USDHC2_DATA2	0x1d4
> +			MX8MP_IOMUXC_SD2_DATA3__USDHC2_DATA3	0x1d4
> +			MX8MP_IOMUXC_GPIO1_IO04__USDHC2_VSELECT	0xc1
> +		>;
> +	};
> +
> +	pinctrl_usdhc2_200mhz: usdhc2-200mhzgrp {
> +		fsl,pins = <
> +			MX8MP_IOMUXC_SD2_CLK__USDHC2_CLK	0x196
> +			MX8MP_IOMUXC_SD2_CMD__USDHC2_CMD	0x1d6
> +			MX8MP_IOMUXC_SD2_DATA0__USDHC2_DATA0	0x1d6
> +			MX8MP_IOMUXC_SD2_DATA1__USDHC2_DATA1	0x1d6
> +			MX8MP_IOMUXC_SD2_DATA2__USDHC2_DATA2	0x1d6
> +			MX8MP_IOMUXC_SD2_DATA3__USDHC2_DATA3	0x1d6
> +			MX8MP_IOMUXC_GPIO1_IO04__USDHC2_VSELECT	0xc1
> +		>;
> +	};
> +};
> diff --git a/arch/arm64/boot/dts/freescale/imx8mp-phycore-som.dtsi b/arch/arm64/boot/dts/freescale/imx8mp-phycore-som.dtsi
> new file mode 100644
> index 000000000000..e1fdfebd8142
> --- /dev/null
> +++ b/arch/arm64/boot/dts/freescale/imx8mp-phycore-som.dtsi
> @@ -0,0 +1,319 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (C) 2020 PHYTEC Messtechnik GmbH
> + * Author: Teresa Remmet <t.remmet@phytec.de>
> + */
> +
> +#include <dt-bindings/net/ti-dp83867.h>
> +#include "imx8mp.dtsi"
> +
> +/ {
> +	model = "PHYTEC phyCORE-i.MX8MP";
> +	compatible = "phytec,imx8mp-phycore-som", "fsl,imx8mp";
> +
> +	aliases {
> +		rtc0 = &rv3028;
> +		rtc1 = &snvs_rtc;
> +	};
> +
> +	memory@40000000 {
> +		device_type = "memory";
> +		reg = <0x0 0x40000000 0 0x80000000>;
> +	};
> +
> +	rtcclkout: rv3028-clkout {

Is it really a separate oscillator giving 32 kHz? Or maybe this is
actually part of PMIC?

In any case node name should reflect class of a device so just "clock".

> +		compatible = "fixed-clock";
> +		#clock-cells = <0>;
> +		clock-frequency = <32768>;
> +		clock-output-names = "rv3028-clkout";
> +		status = "disabled";
> +	};
> +};
> +
> +&A53_0 {
> +	cpu-supply = <&buck2>;
> +};
> +
> +&A53_1 {
> +	cpu-supply = <&buck2>;
> +};
> +
> +&A53_2 {
> +	cpu-supply = <&buck2>;
> +};
> +
> +&A53_3 {
> +	cpu-supply = <&buck2>;
> +};
> +
> +/* ethernet 1 */
> +&fec {
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&pinctrl_fec>;
> +	phy-mode = "rgmii-id";
> +	phy-handle = <&ethphy1>;
> +	fsl,magic-packet;
> +	status = "okay";
> +
> +	mdio {
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +
> +		ethphy1: ethernet-phy@0 {
> +			compatible = "ethernet-phy-ieee802.3-c22";
> +			reg = <0>;
> +			interrupt-parent = <&gpio1>;
> +			interrupts = <15 IRQ_TYPE_EDGE_FALLING>;
> +			ti,rx-internal-delay = <DP83867_RGMIIDCTL_2_00_NS>;
> +			ti,tx-internal-delay = <DP83867_RGMIIDCTL_2_00_NS>;
> +			ti,fifo-depth = <DP83867_PHYCR_FIFO_DEPTH_4_B_NIB>;
> +			ti,clk-output-sel = <DP83867_CLK_O_SEL_OFF>;
> +			enet-phy-lane-no-swap;
> +		};
> +	};
> +};
> +
> +&i2c1 {
> +	clock-frequency = <400000>;
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&pinctrl_i2c1>;
> +	pinctrl-1 = <&pinctrl_i2c1_gpio>;
> +	sda-gpios = <&gpio5 15 (GPIO_ACTIVE_HIGH | GPIO_OPEN_DRAIN)>;
> +	scl-gpios = <&gpio5 14 (GPIO_ACTIVE_HIGH | GPIO_OPEN_DRAIN)>;
> +	status = "okay";
> +
> +	pmic: pca9450@25 {

Node name: pmic.

> +		reg = <0x25>;
> +		compatible = "nxp,pca9450c";
> +		pinctrl-names = "default";
> +		pinctrl-0 = <&pinctrl_pmic>;
> +		interrupt-parent = <&gpio4>;
> +		interrupts = <18 IRQ_TYPE_LEVEL_LOW>;
> +
> +		regulators {
> +			#address-cells = <1>;
> +			#size-cells = <0>;
> +
> +			buck1: BUCK1 {
> +				reg = <0>;

Does this all pass dtc W=1 builds and dtschema? The node name does not
have unit address. Bindings explicitly do not allow it... This looks
wrong.

> +				regulator-compatible = "BUCK1";
> +				regulator-min-microvolt = <600000>;
> +				regulator-max-microvolt = <2187500>;
> +				regulator-boot-on;
> +				regulator-always-on;
> +				regulator-ramp-delay = <3125>;
> +			};
> +
> +			buck2: BUCK2 {
> +				reg = <1>;
> +				regulator-compatible = "BUCK2";
> +				regulator-min-microvolt = <600000>;
> +				regulator-max-microvolt = <2187500>;
> +				regulator-boot-on;
> +				regulator-always-on;
> +				regulator-ramp-delay = <3125>;
> +			};
> +
> +			buck4: BUCK4 {
> +				reg = <3>;
> +				regulator-compatible = "BUCK4";
> +				regulator-min-microvolt = <600000>;
> +				regulator-max-microvolt = <3400000>;
> +				regulator-boot-on;
> +				regulator-always-on;
> +			};
> +
> +			buck5: BUCK5 {
> +				reg = <4>;
> +				regulator-compatible = "BUCK5";
> +				regulator-min-microvolt = <600000>;
> +				regulator-max-microvolt = <3400000>;
> +				regulator-boot-on;
> +				regulator-always-on;
> +			};
> +
> +			buck6: BUCK6 {
> +				reg = <5>;
> +				regulator-compatible = "BUCK6";
> +				regulator-min-microvolt = <600000>;
> +				regulator-max-microvolt = <3400000>;
> +				regulator-boot-on;
> +				regulator-always-on;
> +			};
> +
> +			ldo1: LDO1 {
> +				reg = <6>;
> +				regulator-compatible = "LDO1";
> +				regulator-min-microvolt = <1600000>;
> +				regulator-max-microvolt = <3300000>;
> +				regulator-boot-on;
> +				regulator-always-on;
> +			};
> +
> +			ldo2: LDO2 {
> +				reg = <7>;
> +				regulator-compatible = "LDO2";
> +				regulator-min-microvolt = <800000>;
> +				regulator-max-microvolt = <1150000>;
> +				regulator-boot-on;
> +				regulator-always-on;
> +			};
> +
> +			ldo3: LDO3 {
> +				reg = <8>;
> +				regulator-compatible = "LDO3";
> +				regulator-min-microvolt = <800000>;
> +				regulator-max-microvolt = <3300000>;
> +				regulator-boot-on;
> +				regulator-always-on;
> +			};
> +
> +			ldo4: LDO4 {
> +				reg = <9>;
> +				regulator-compatible = "LDO4";
> +				regulator-min-microvolt = <800000>;
> +				regulator-max-microvolt = <3300000>;
> +				regulator-boot-on;
> +				regulator-always-on;
> +			};
> +
> +			ldo5: LDO5 {
> +				reg = <10>;
> +				regulator-compatible = "LDO5";
> +				regulator-min-microvolt = <1800000>;
> +				regulator-max-microvolt = <3300000>;
> +			};
> +		};
> +	};
> +
> +	eeprom@51 {
> +		compatible = "atmel,24c32";
> +		reg = <0x51>;
> +		pagesize = <32>;
> +		status = "okay";
> +	};
> +
> +	rv3028: rtc@52 {
> +		compatible = "microcrystal,rv3028";
> +		reg = <0x52>;
> +		trickle-resistor-ohms = <1000>;
> +		enable-level-switching-mode;
> +		clocks = <&rtcclkout>;
> +		status = "okay";
> +	};
> +};
> +
> +/* eMMC */
> +&usdhc3 {
> +	pinctrl-names = "default", "state_100mhz", "state_200mhz";
> +	pinctrl-0 = <&pinctrl_usdhc3>;
> +	pinctrl-1 = <&pinctrl_usdhc3_100mhz>;
> +	pinctrl-2 = <&pinctrl_usdhc3_200mhz>;
> +	bus-width = <8>;
> +	non-removable;
> +	status = "okay";
> +};
> +
> +&wdog1 {
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&pinctrl_wdog>;
> +	fsl,ext-reset-output;
> +	status = "okay";
> +};
> +
> +&iomuxc {
> +	pinctrl_fec: fecgrp {
> +		fsl,pins = <
> +			MX8MP_IOMUXC_SAI1_RXD2__ENET1_MDC		0x3
> +			MX8MP_IOMUXC_SAI1_RXD3__ENET1_MDIO		0x3
> +			MX8MP_IOMUXC_SAI1_RXD4__ENET1_RGMII_RD0		0x91
> +			MX8MP_IOMUXC_SAI1_RXD5__ENET1_RGMII_RD1		0x91
> +			MX8MP_IOMUXC_SAI1_RXD6__ENET1_RGMII_RD2		0x91
> +			MX8MP_IOMUXC_SAI1_RXD7__ENET1_RGMII_RD3		0x91
> +			MX8MP_IOMUXC_SAI1_TXC__ENET1_RGMII_RXC		0x91
> +			MX8MP_IOMUXC_SAI1_TXFS__ENET1_RGMII_RX_CTL	0x91
> +			MX8MP_IOMUXC_SAI1_TXD0__ENET1_RGMII_TD0		0x1f
> +			MX8MP_IOMUXC_SAI1_TXD1__ENET1_RGMII_TD1		0x1f
> +			MX8MP_IOMUXC_SAI1_TXD2__ENET1_RGMII_TD2		0x1f
> +			MX8MP_IOMUXC_SAI1_TXD3__ENET1_RGMII_TD3		0x1f
> +			MX8MP_IOMUXC_SAI1_TXD4__ENET1_RGMII_TX_CTL	0x1f
> +			MX8MP_IOMUXC_SAI1_TXD5__ENET1_RGMII_TXC		0x1f
> +			MX8MP_IOMUXC_GPIO1_IO15__GPIO1_IO15		0x11
> +		>;
> +	};
> +
> +	pinctrl_i2c1: i2c1grp {
> +		fsl,pins = <
> +			MX8MP_IOMUXC_I2C1_SCL__I2C1_SCL		0x400001c3
> +			MX8MP_IOMUXC_I2C1_SDA__I2C1_SDA		0x400001c3
> +		>;
> +	};
> +
> +	pinctrl_i2c1_gpio: i2c1gpiogrp {
> +		fsl,pins = <
> +			MX8MP_IOMUXC_I2C1_SCL__GPIO5_IO14	0x1e3
> +			MX8MP_IOMUXC_I2C1_SDA__GPIO5_IO15	0x1e3
> +		>;
> +	};
> +
> +

Too many line breaks.

Best regards,
Krzysztof

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 4/4] arm64: dts: freescale: Add support for phyBOARD-Pollux-i.MX8MP

[Teresa Remmet]

Hello Krzysztof,

Am Montag, den 07.12.2020, 13:09 +0100 schrieb Krzysztof Kozlowski:
> On Fri, Dec 04, 2020 at 09:33:02PM +0100, Teresa Remmet wrote:
> > Add initial support for phyBOARD-Pollux-i.MX8MP.
> > Supported basic features:
> > 	* eMMC
> > 	* i2c EEPROM
> > 	* i2c RTC
> > 	* i2c LED
> > 	* PMIC
> > 	* debug UART
> > 	* SD card
> > 	* 1Gbit Ethernet (fec)
> > 	* watchdog
> > 
> > Signed-off-by: Teresa Remmet <t.remmet@phytec.de>
> > ---
> >  arch/arm64/boot/dts/freescale/Makefile             |   1 +
> >  .../dts/freescale/imx8mp-phyboard-pollux-rdk.dts   |  16 ++
> >  .../boot/dts/freescale/imx8mp-phyboard-pollux.dtsi | 152
> > ++++++++++
> >  .../boot/dts/freescale/imx8mp-phycore-som.dtsi     | 319
> > +++++++++++++++++++++
> >  4 files changed, 488 insertions(+)
> >  create mode 100644 arch/arm64/boot/dts/freescale/imx8mp-phyboard-
> > pollux-rdk.dts
> >  create mode 100644 arch/arm64/boot/dts/freescale/imx8mp-phyboard-
> > pollux.dtsi
> >  create mode 100644 arch/arm64/boot/dts/freescale/imx8mp-phycore-
> > som.dtsi
> > 
> > diff --git a/arch/arm64/boot/dts/freescale/Makefile
> > b/arch/arm64/boot/dts/freescale/Makefile
> > index acfb8af45912..a43b496678be 100644
> > --- a/arch/arm64/boot/dts/freescale/Makefile
> > +++ b/arch/arm64/boot/dts/freescale/Makefile
> > @@ -37,6 +37,7 @@ dtb-$(CONFIG_ARCH_MXC) += imx8mn-evk.dtb
> >  dtb-$(CONFIG_ARCH_MXC) += imx8mn-ddr4-evk.dtb
> >  dtb-$(CONFIG_ARCH_MXC) += imx8mn-var-som-symphony.dtb
> >  dtb-$(CONFIG_ARCH_MXC) += imx8mp-evk.dtb
> > +dtb-$(CONFIG_ARCH_MXC) += imx8mp-phyboard-pollux-rdk.dtb
> >  dtb-$(CONFIG_ARCH_MXC) += imx8mq-evk.dtb
> >  dtb-$(CONFIG_ARCH_MXC) += imx8mq-hummingboard-pulse.dtb
> >  dtb-$(CONFIG_ARCH_MXC) += imx8mq-librem5-devkit.dtb
> > diff --git a/arch/arm64/boot/dts/freescale/imx8mp-phyboard-pollux-
> > rdk.dts b/arch/arm64/boot/dts/freescale/imx8mp-phyboard-pollux-
> > rdk.dts
> > new file mode 100644
> > index 000000000000..dd64be32c99d
> > --- /dev/null
> > +++ b/arch/arm64/boot/dts/freescale/imx8mp-phyboard-pollux-rdk.dts
> > @@ -0,0 +1,16 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Copyright (C) 2020 PHYTEC Messtechnik GmbH
> > + * Author: Teresa Remmet <t.remmet@phytec.de>
> > + */
> > +
> > +/dts-v1/;
> > +
> > +#include "imx8mp-phycore-som.dtsi"
> > +#include "imx8mp-phyboard-pollux.dtsi"
> > +
> > +/ {
> > +	model = "PHYTEC phyBOARD-Pollux i.MX8MP";
> > +	compatible = "phytec,imx8mp-phyboard-pollux-rdk",
> > +		     "phytec,imx8mp-phycore-som", "fsl,imx8mp";
> 
> This is the purpose of this file? Why having a DTS to include DTSI
> only?
> Usually there is just DTSI for SOM and DTS fot the board.

we have different options for the SoMs. Like SPI-NOR flash mounted or
not. We usually add this to the SoM include, but disable it. We enable
this in the dts if mounted. This makes it easy to generate different
device trees for different SoM options. So far upstream is not every
feature supported. So we don't do anything in the dts yet. But I want
to setup the layout already.

I hope this makes it clear.

> 
> > +};
> > diff --git a/arch/arm64/boot/dts/freescale/imx8mp-phyboard-
> > pollux.dtsi b/arch/arm64/boot/dts/freescale/imx8mp-phyboard-
> > pollux.dtsi
> > new file mode 100644
> > index 000000000000..dbeaa27eb043
> > --- /dev/null
> > +++ b/arch/arm64/boot/dts/freescale/imx8mp-phyboard-pollux.dtsi
> > @@ -0,0 +1,152 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Copyright (C) 2020 PHYTEC Messtechnik GmbH
> > + * Author: Teresa Remmet <t.remmet@phytec.de>
> > + */
> > +
> > +#include <dt-bindings/leds/leds-pca9532.h>
> > +#include <dt-bindings/pwm/pwm.h>
> > +
> > +/ {
> > +	chosen {
> > +		stdout-path = &uart2;
> > +	};
> > +
> > +	reg_usdhc2_vmmc: regulator-usdhc2 {
> > +		compatible = "regulator-fixed";
> > +		regulator-name = "VSD_3V3";
> > +		regulator-min-microvolt = <3300000>;
> > +		regulator-max-microvolt = <3300000>;
> > +		gpio = <&gpio2 19 GPIO_ACTIVE_HIGH>;
> > +		enable-active-high;
> > +		startup-delay-us = <100>;
> > +		off-on-delay-us = <12000>;
> > +	};
> > +};
> > +
> > +&i2c2 {
> > +	clock-frequency = <400000>;
> > +	pinctrl-names = "default";
> > +	pinctrl-0 = <&pinctrl_i2c2>;
> > +	pinctrl-1 = <&pinctrl_i2c2_gpio>;
> > +	sda-gpios = <&gpio5 17 (GPIO_ACTIVE_HIGH | GPIO_OPEN_DRAIN)>;
> > +	scl-gpios = <&gpio5 16 (GPIO_ACTIVE_HIGH | GPIO_OPEN_DRAIN)>;
> > +	status = "okay";
> > +
> > +	eeprom@51 {
> > +		compatible = "atmel,24c02";
> > +		reg = <0x51>;
> > +		pagesize = <16>;
> > +		status = "okay";
> > +	};
> > +
> > +	leddimmer@62 {
> 
> I think name "leds" is more appropriate.

okay I will change it.

> 
> > +		compatible = "nxp,pca9533";
> > +		reg = <0x62>;
> > +		status = "okay";
> > +
> > +		led1 {
> > +			label = "red:user1";
> > +			type = <PCA9532_TYPE_LED>;
> > +		};
> > +
> > +		led2 {
> > +			label = "green:user2";
> > +			type = <PCA9532_TYPE_LED>;
> > +		};
> > +
> > +		led3 {
> > +			label = "blue:user3";
> > +			type = <PCA9532_TYPE_LED>;
> > +		};
> > +	};
> > +};
> > +
> > +&snvs_pwrkey {
> > +	status = "okay";
> > +};
> > +
> > +/* debug console */
> > +&uart2 {
> > +	pinctrl-names = "default";
> > +	pinctrl-0 = <&pinctrl_uart2>;
> > +	status = "okay";
> > +};
> > +
> > +/* SD-Card */
> > +&usdhc2 {
> > +	pinctrl-names = "default", "state_100mhz", "state_200mhz";
> > +	pinctrl-0 = <&pinctrl_usdhc2>, <&pinctrl_usdhc2_pins>;
> > +	pinctrl-1 = <&pinctrl_usdhc2_100mhz>, <&pinctrl_usdhc2_pins>;
> > +	pinctrl-2 = <&pinctrl_usdhc2_200mhz>, <&pinctrl_usdhc2_pins>;
> > +	cd-gpios = <&gpio2 12 GPIO_ACTIVE_LOW>;
> > +	vmmc-supply = <&reg_usdhc2_vmmc>;
> > +	bus-width = <4>;
> > +	status = "okay";
> > +};
> > +
> > +&iomuxc {
> > +	pinctrl_i2c2: i2c2grp {
> > +		fsl,pins = <
> > +			MX8MP_IOMUXC_I2C2_SCL__I2C2_SCL		0x4
> > 00001c3
> > +			MX8MP_IOMUXC_I2C2_SDA__I2C2_SDA		0x4
> > 00001c3
> > +		>;
> > +	};
> > +
> > +	pinctrl_i2c2_gpio: i2c2gpiogrp {
> > +		fsl,pins = <
> > +			MX8MP_IOMUXC_I2C2_SCL__GPIO5_IO16	0x1e3
> > +			MX8MP_IOMUXC_I2C2_SDA__GPIO5_IO17	0x1e3
> > +		>;
> > +	};
> > +
> > +	pinctrl_uart2: uart2grp {
> > +		fsl,pins = <
> > +			MX8MP_IOMUXC_UART2_RXD__UART2_DCE_RX	0x4
> > 9
> > +			MX8MP_IOMUXC_UART2_TXD__UART2_DCE_TX	0x4
> > 9
> > +		>;
> > +	};
> > +
> > +	pinctrl_usdhc2_pins: usdhc2-gpiogrp {
> > +		fsl,pins = <
> > +			MX8MP_IOMUXC_SD2_CD_B__GPIO2_IO12	0x1c4
> > +			MX8MP_IOMUXC_SD2_RESET_B__GPIO2_IO19	0x4
> > 1
> > +		>;
> > +	};
> > +
> > +	pinctrl_usdhc2: usdhc2grp {
> > +		fsl,pins = <
> > +			MX8MP_IOMUXC_SD2_CLK__USDHC2_CLK	0x190
> > +			MX8MP_IOMUXC_SD2_CMD__USDHC2_CMD	0x1d0
> > +			MX8MP_IOMUXC_SD2_DATA0__USDHC2_DATA0	0x1
> > d0
> > +			MX8MP_IOMUXC_SD2_DATA1__USDHC2_DATA1	0x1
> > d0
> > +			MX8MP_IOMUXC_SD2_DATA2__USDHC2_DATA2	0x1
> > d0
> > +			MX8MP_IOMUXC_SD2_DATA3__USDHC2_DATA3	0x1
> > d0
> > +			MX8MP_IOMUXC_GPIO1_IO04__USDHC2_VSELECT	0xc
> > 1
> > +		>;
> > +	};
> > +
> > +	pinctrl_usdhc2_100mhz: usdhc2-100mhzgrp {
> > +		fsl,pins = <
> > +			MX8MP_IOMUXC_SD2_CLK__USDHC2_CLK	0x194
> > +			MX8MP_IOMUXC_SD2_CMD__USDHC2_CMD	0x1d4
> > +			MX8MP_IOMUXC_SD2_DATA0__USDHC2_DATA0	0x1
> > d4
> > +			MX8MP_IOMUXC_SD2_DATA1__USDHC2_DATA1	0x1
> > d4
> > +			MX8MP_IOMUXC_SD2_DATA2__USDHC2_DATA2	0x1
> > d4
> > +			MX8MP_IOMUXC_SD2_DATA3__USDHC2_DATA3	0x1
> > d4
> > +			MX8MP_IOMUXC_GPIO1_IO04__USDHC2_VSELECT	0xc
> > 1
> > +		>;
> > +	};
> > +
> > +	pinctrl_usdhc2_200mhz: usdhc2-200mhzgrp {
> > +		fsl,pins = <
> > +			MX8MP_IOMUXC_SD2_CLK__USDHC2_CLK	0x196
> > +			MX8MP_IOMUXC_SD2_CMD__USDHC2_CMD	0x1d6
> > +			MX8MP_IOMUXC_SD2_DATA0__USDHC2_DATA0	0x1
> > d6
> > +			MX8MP_IOMUXC_SD2_DATA1__USDHC2_DATA1	0x1
> > d6
> > +			MX8MP_IOMUXC_SD2_DATA2__USDHC2_DATA2	0x1
> > d6
> > +			MX8MP_IOMUXC_SD2_DATA3__USDHC2_DATA3	0x1
> > d6
> > +			MX8MP_IOMUXC_GPIO1_IO04__USDHC2_VSELECT	0xc
> > 1
> > +		>;
> > +	};
> > +};
> > diff --git a/arch/arm64/boot/dts/freescale/imx8mp-phycore-som.dtsi
> > b/arch/arm64/boot/dts/freescale/imx8mp-phycore-som.dtsi
> > new file mode 100644
> > index 000000000000..e1fdfebd8142
> > --- /dev/null
> > +++ b/arch/arm64/boot/dts/freescale/imx8mp-phycore-som.dtsi
> > @@ -0,0 +1,319 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Copyright (C) 2020 PHYTEC Messtechnik GmbH
> > + * Author: Teresa Remmet <t.remmet@phytec.de>
> > + */
> > +
> > +#include <dt-bindings/net/ti-dp83867.h>
> > +#include "imx8mp.dtsi"
> > +
> > +/ {
> > +	model = "PHYTEC phyCORE-i.MX8MP";
> > +	compatible = "phytec,imx8mp-phycore-som", "fsl,imx8mp";
> > +
> > +	aliases {
> > +		rtc0 = &rv3028;
> > +		rtc1 = &snvs_rtc;
> > +	};
> > +
> > +	memory@40000000 {
> > +		device_type = "memory";
> > +		reg = <0x0 0x40000000 0 0x80000000>;
> > +	};
> > +
> > +	rtcclkout: rv3028-clkout {
> 
> Is it really a separate oscillator giving 32 kHz? Or maybe this is
> actually part of PMIC?

It is a clock out of the used i2c rtc. Which I actually trying to
disable. As it is not connected. But it is enabled as default.

> 
> In any case node name should reflect class of a device so just
> "clock".

Changing that.

> 
> > +		compatible = "fixed-clock";
> > +		#clock-cells = <0>;
> > +		clock-frequency = <32768>;
> > +		clock-output-names = "rv3028-clkout";
> > +		status = "disabled";
> > +	};
> > +};
> > +
> > +&A53_0 {
> > +	cpu-supply = <&buck2>;
> > +};
> > +
> > +&A53_1 {
> > +	cpu-supply = <&buck2>;
> > +};
> > +
> > +&A53_2 {
> > +	cpu-supply = <&buck2>;
> > +};
> > +
> > +&A53_3 {
> > +	cpu-supply = <&buck2>;
> > +};
> > +
> > +/* ethernet 1 */
> > +&fec {
> > +	pinctrl-names = "default";
> > +	pinctrl-0 = <&pinctrl_fec>;
> > +	phy-mode = "rgmii-id";
> > +	phy-handle = <&ethphy1>;
> > +	fsl,magic-packet;
> > +	status = "okay";
> > +
> > +	mdio {
> > +		#address-cells = <1>;
> > +		#size-cells = <0>;
> > +
> > +		ethphy1: ethernet-phy@0 {
> > +			compatible = "ethernet-phy-ieee802.3-c22";
> > +			reg = <0>;
> > +			interrupt-parent = <&gpio1>;
> > +			interrupts = <15 IRQ_TYPE_EDGE_FALLING>;
> > +			ti,rx-internal-delay =
> > <DP83867_RGMIIDCTL_2_00_NS>;
> > +			ti,tx-internal-delay =
> > <DP83867_RGMIIDCTL_2_00_NS>;
> > +			ti,fifo-depth =
> > <DP83867_PHYCR_FIFO_DEPTH_4_B_NIB>;
> > +			ti,clk-output-sel = <DP83867_CLK_O_SEL_OFF>;
> > +			enet-phy-lane-no-swap;
> > +		};
> > +	};
> > +};
> > +
> > +&i2c1 {
> > +	clock-frequency = <400000>;
> > +	pinctrl-names = "default";
> > +	pinctrl-0 = <&pinctrl_i2c1>;
> > +	pinctrl-1 = <&pinctrl_i2c1_gpio>;
> > +	sda-gpios = <&gpio5 15 (GPIO_ACTIVE_HIGH | GPIO_OPEN_DRAIN)>;
> > +	scl-gpios = <&gpio5 14 (GPIO_ACTIVE_HIGH | GPIO_OPEN_DRAIN)>;
> > +	status = "okay";
> > +
> > +	pmic: pca9450@25 {
> 
> Node name: pmic.

Will be fixed.

> 
> > +		reg = <0x25>;
> > +		compatible = "nxp,pca9450c";
> > +		pinctrl-names = "default";
> > +		pinctrl-0 = <&pinctrl_pmic>;
> > +		interrupt-parent = <&gpio4>;
> > +		interrupts = <18 IRQ_TYPE_LEVEL_LOW>;
> > +
> > +		regulators {
> > +			#address-cells = <1>;
> > +			#size-cells = <0>;
> > +
> > +			buck1: BUCK1 {
> > +				reg = <0>;
> 
> Does this all pass dtc W=1 builds and dtschema? The node name does
> not
> have unit address. Bindings explicitly do not allow it... This looks
> wrong.

The reg property is a leftover and not correct. I will remove this and
others below. Same with #address-cells and #size-cells.

> 
> > +				regulator-compatible = "BUCK1";
> > +				regulator-min-microvolt = <600000>;
> > +				regulator-max-microvolt = <2187500>;
> > +				regulator-boot-on;
> > +				regulator-always-on;
> > +				regulator-ramp-delay = <3125>;
> > +			};
> > +
> > +			buck2: BUCK2 {
> > +				reg = <1>;
> > +				regulator-compatible = "BUCK2";
> > +				regulator-min-microvolt = <600000>;
> > +				regulator-max-microvolt = <2187500>;
> > +				regulator-boot-on;
> > +				regulator-always-on;
> > +				regulator-ramp-delay = <3125>;
> > +			};
> > +
> > +			buck4: BUCK4 {
> > +				reg = <3>;
> > +				regulator-compatible = "BUCK4";
> > +				regulator-min-microvolt = <600000>;
> > +				regulator-max-microvolt = <3400000>;
> > +				regulator-boot-on;
> > +				regulator-always-on;
> > +			};
> > +
> > +			buck5: BUCK5 {
> > +				reg = <4>;
> > +				regulator-compatible = "BUCK5";
> > +				regulator-min-microvolt = <600000>;
> > +				regulator-max-microvolt = <3400000>;
> > +				regulator-boot-on;
> > +				regulator-always-on;
> > +			};
> > +
> > +			buck6: BUCK6 {
> > +				reg = <5>;
> > +				regulator-compatible = "BUCK6";
> > +				regulator-min-microvolt = <600000>;
> > +				regulator-max-microvolt = <3400000>;
> > +				regulator-boot-on;
> > +				regulator-always-on;
> > +			};
> > +
> > +			ldo1: LDO1 {
> > +				reg = <6>;
> > +				regulator-compatible = "LDO1";
> > +				regulator-min-microvolt = <1600000>;
> > +				regulator-max-microvolt = <3300000>;
> > +				regulator-boot-on;
> > +				regulator-always-on;
> > +			};
> > +
> > +			ldo2: LDO2 {
> > +				reg = <7>;
> > +				regulator-compatible = "LDO2";
> > +				regulator-min-microvolt = <800000>;
> > +				regulator-max-microvolt = <1150000>;
> > +				regulator-boot-on;
> > +				regulator-always-on;
> > +			};
> > +
> > +			ldo3: LDO3 {
> > +				reg = <8>;
> > +				regulator-compatible = "LDO3";
> > +				regulator-min-microvolt = <800000>;
> > +				regulator-max-microvolt = <3300000>;
> > +				regulator-boot-on;
> > +				regulator-always-on;
> > +			};
> > +
> > +			ldo4: LDO4 {
> > +				reg = <9>;
> > +				regulator-compatible = "LDO4";
> > +				regulator-min-microvolt = <800000>;
> > +				regulator-max-microvolt = <3300000>;
> > +				regulator-boot-on;
> > +				regulator-always-on;
> > +			};
> > +
> > +			ldo5: LDO5 {
> > +				reg = <10>;
> > +				regulator-compatible = "LDO5";
> > +				regulator-min-microvolt = <1800000>;
> > +				regulator-max-microvolt = <3300000>;
> > +			};
> > +		};
> > +	};
> > +
> > +	eeprom@51 {
> > +		compatible = "atmel,24c32";
> > +		reg = <0x51>;
> > +		pagesize = <32>;
> > +		status = "okay";
> > +	};
> > +
> > +	rv3028: rtc@52 {
> > +		compatible = "microcrystal,rv3028";
> > +		reg = <0x52>;
> > +		trickle-resistor-ohms = <1000>;
> > +		enable-level-switching-mode;
> > +		clocks = <&rtcclkout>;
> > +		status = "okay";
> > +	};
> > +};
> > +
> > +/* eMMC */
> > +&usdhc3 {
> > +	pinctrl-names = "default", "state_100mhz", "state_200mhz";
> > +	pinctrl-0 = <&pinctrl_usdhc3>;
> > +	pinctrl-1 = <&pinctrl_usdhc3_100mhz>;
> > +	pinctrl-2 = <&pinctrl_usdhc3_200mhz>;
> > +	bus-width = <8>;
> > +	non-removable;
> > +	status = "okay";
> > +};
> > +
> > +&wdog1 {
> > +	pinctrl-names = "default";
> > +	pinctrl-0 = <&pinctrl_wdog>;
> > +	fsl,ext-reset-output;
> > +	status = "okay";
> > +};
> > +
> > +&iomuxc {
> > +	pinctrl_fec: fecgrp {
> > +		fsl,pins = <
> > +			MX8MP_IOMUXC_SAI1_RXD2__ENET1_MDC		0x3
> > +			MX8MP_IOMUXC_SAI1_RXD3__ENET1_MDIO		0x3
> > +			MX8MP_IOMUXC_SAI1_RXD4__ENET1_RGMII_RD0		
> > 0x91
> > +			MX8MP_IOMUXC_SAI1_RXD5__ENET1_RGMII_RD1		
> > 0x91
> > +			MX8MP_IOMUXC_SAI1_RXD6__ENET1_RGMII_RD2		
> > 0x91
> > +			MX8MP_IOMUXC_SAI1_RXD7__ENET1_RGMII_RD3		
> > 0x91
> > +			MX8MP_IOMUXC_SAI1_TXC__ENET1_RGMII_RXC		
> > 0x91
> > +			MX8MP_IOMUXC_SAI1_TXFS__ENET1_RGMII_RX_CTL	0x9
> > 1
> > +			MX8MP_IOMUXC_SAI1_TXD0__ENET1_RGMII_TD0		
> > 0x1f
> > +			MX8MP_IOMUXC_SAI1_TXD1__ENET1_RGMII_TD1		
> > 0x1f
> > +			MX8MP_IOMUXC_SAI1_TXD2__ENET1_RGMII_TD2		
> > 0x1f
> > +			MX8MP_IOMUXC_SAI1_TXD3__ENET1_RGMII_TD3		
> > 0x1f
> > +			MX8MP_IOMUXC_SAI1_TXD4__ENET1_RGMII_TX_CTL	0x1
> > f
> > +			MX8MP_IOMUXC_SAI1_TXD5__ENET1_RGMII_TXC		
> > 0x1f
> > +			MX8MP_IOMUXC_GPIO1_IO15__GPIO1_IO15		0x1
> > 1
> > +		>;
> > +	};
> > +
> > +	pinctrl_i2c1: i2c1grp {
> > +		fsl,pins = <
> > +			MX8MP_IOMUXC_I2C1_SCL__I2C1_SCL		0x4
> > 00001c3
> > +			MX8MP_IOMUXC_I2C1_SDA__I2C1_SDA		0x4
> > 00001c3
> > +		>;
> > +	};
> > +
> > +	pinctrl_i2c1_gpio: i2c1gpiogrp {
> > +		fsl,pins = <
> > +			MX8MP_IOMUXC_I2C1_SCL__GPIO5_IO14	0x1e3
> > +			MX8MP_IOMUXC_I2C1_SDA__GPIO5_IO15	0x1e3
> > +		>;
> > +	};
> > +
> > +
> 
> Too many line breaks.

Oh, I will remove this.

Thank you for the review.

Teresa
> 
> Best regards,
> Krzysztof

Re: [PATCH 4/4] arm64: dts: freescale: Add support for phyBOARD-Pollux-i.MX8MP

[Teresa Remmet]

Hello Krzysztof,

Am Montag, den 07.12.2020, 13:09 +0100 schrieb Krzysztof Kozlowski:
> On Fri, Dec 04, 2020 at 09:33:02PM +0100, Teresa Remmet wrote:
> > Add initial support for phyBOARD-Pollux-i.MX8MP.
> > Supported basic features:
> > 	* eMMC
> > 	* i2c EEPROM
> > 	* i2c RTC
> > 	* i2c LED
> > 	* PMIC
> > 	* debug UART
> > 	* SD card
> > 	* 1Gbit Ethernet (fec)
> > 	* watchdog
> > 
> > Signed-off-by: Teresa Remmet <t.remmet@phytec.de>
> > ---
> >  arch/arm64/boot/dts/freescale/Makefile             |   1 +
> >  .../dts/freescale/imx8mp-phyboard-pollux-rdk.dts   |  16 ++
> >  .../boot/dts/freescale/imx8mp-phyboard-pollux.dtsi | 152
> > ++++++++++
> >  .../boot/dts/freescale/imx8mp-phycore-som.dtsi     | 319
> > +++++++++++++++++++++
> >  4 files changed, 488 insertions(+)
> >  create mode 100644 arch/arm64/boot/dts/freescale/imx8mp-phyboard-
> > pollux-rdk.dts
> >  create mode 100644 arch/arm64/boot/dts/freescale/imx8mp-phyboard-
> > pollux.dtsi
> >  create mode 100644 arch/arm64/boot/dts/freescale/imx8mp-phycore-
> > som.dtsi
> > 
> > diff --git a/arch/arm64/boot/dts/freescale/Makefile
> > b/arch/arm64/boot/dts/freescale/Makefile
> > index acfb8af45912..a43b496678be 100644
> > --- a/arch/arm64/boot/dts/freescale/Makefile
> > +++ b/arch/arm64/boot/dts/freescale/Makefile
> > @@ -37,6 +37,7 @@ dtb-$(CONFIG_ARCH_MXC) += imx8mn-evk.dtb
> >  dtb-$(CONFIG_ARCH_MXC) += imx8mn-ddr4-evk.dtb
> >  dtb-$(CONFIG_ARCH_MXC) += imx8mn-var-som-symphony.dtb
> >  dtb-$(CONFIG_ARCH_MXC) += imx8mp-evk.dtb
> > +dtb-$(CONFIG_ARCH_MXC) += imx8mp-phyboard-pollux-rdk.dtb
> >  dtb-$(CONFIG_ARCH_MXC) += imx8mq-evk.dtb
> >  dtb-$(CONFIG_ARCH_MXC) += imx8mq-hummingboard-pulse.dtb
> >  dtb-$(CONFIG_ARCH_MXC) += imx8mq-librem5-devkit.dtb
> > diff --git a/arch/arm64/boot/dts/freescale/imx8mp-phyboard-pollux-
> > rdk.dts b/arch/arm64/boot/dts/freescale/imx8mp-phyboard-pollux-
> > rdk.dts
> > new file mode 100644
> > index 000000000000..dd64be32c99d
> > --- /dev/null
> > +++ b/arch/arm64/boot/dts/freescale/imx8mp-phyboard-pollux-rdk.dts
> > @@ -0,0 +1,16 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Copyright (C) 2020 PHYTEC Messtechnik GmbH
> > + * Author: Teresa Remmet <t.remmet@phytec.de>
> > + */
> > +
> > +/dts-v1/;
> > +
> > +#include "imx8mp-phycore-som.dtsi"
> > +#include "imx8mp-phyboard-pollux.dtsi"
> > +
> > +/ {
> > +	model = "PHYTEC phyBOARD-Pollux i.MX8MP";
> > +	compatible = "phytec,imx8mp-phyboard-pollux-rdk",
> > +		     "phytec,imx8mp-phycore-som", "fsl,imx8mp";
> 
> This is the purpose of this file? Why having a DTS to include DTSI
> only?
> Usually there is just DTSI for SOM and DTS fot the board.

we have different options for the SoMs. Like SPI-NOR flash mounted or
not. We usually add this to the SoM include, but disable it. We enable
this in the dts if mounted. This makes it easy to generate different
device trees for different SoM options. So far upstream is not every
feature supported. So we don't do anything in the dts yet. But I want
to setup the layout already.

I hope this makes it clear.

> 
> > +};
> > diff --git a/arch/arm64/boot/dts/freescale/imx8mp-phyboard-
> > pollux.dtsi b/arch/arm64/boot/dts/freescale/imx8mp-phyboard-
> > pollux.dtsi
> > new file mode 100644
> > index 000000000000..dbeaa27eb043
> > --- /dev/null
> > +++ b/arch/arm64/boot/dts/freescale/imx8mp-phyboard-pollux.dtsi
> > @@ -0,0 +1,152 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Copyright (C) 2020 PHYTEC Messtechnik GmbH
> > + * Author: Teresa Remmet <t.remmet@phytec.de>
> > + */
> > +
> > +#include <dt-bindings/leds/leds-pca9532.h>
> > +#include <dt-bindings/pwm/pwm.h>
> > +
> > +/ {
> > +	chosen {
> > +		stdout-path = &uart2;
> > +	};
> > +
> > +	reg_usdhc2_vmmc: regulator-usdhc2 {
> > +		compatible = "regulator-fixed";
> > +		regulator-name = "VSD_3V3";
> > +		regulator-min-microvolt = <3300000>;
> > +		regulator-max-microvolt = <3300000>;
> > +		gpio = <&gpio2 19 GPIO_ACTIVE_HIGH>;
> > +		enable-active-high;
> > +		startup-delay-us = <100>;
> > +		off-on-delay-us = <12000>;
> > +	};
> > +};
> > +
> > +&i2c2 {
> > +	clock-frequency = <400000>;
> > +	pinctrl-names = "default";
> > +	pinctrl-0 = <&pinctrl_i2c2>;
> > +	pinctrl-1 = <&pinctrl_i2c2_gpio>;
> > +	sda-gpios = <&gpio5 17 (GPIO_ACTIVE_HIGH | GPIO_OPEN_DRAIN)>;
> > +	scl-gpios = <&gpio5 16 (GPIO_ACTIVE_HIGH | GPIO_OPEN_DRAIN)>;
> > +	status = "okay";
> > +
> > +	eeprom@51 {
> > +		compatible = "atmel,24c02";
> > +		reg = <0x51>;
> > +		pagesize = <16>;
> > +		status = "okay";
> > +	};
> > +
> > +	leddimmer@62 {
> 
> I think name "leds" is more appropriate.

okay I will change it.

> 
> > +		compatible = "nxp,pca9533";
> > +		reg = <0x62>;
> > +		status = "okay";
> > +
> > +		led1 {
> > +			label = "red:user1";
> > +			type = <PCA9532_TYPE_LED>;
> > +		};
> > +
> > +		led2 {
> > +			label = "green:user2";
> > +			type = <PCA9532_TYPE_LED>;
> > +		};
> > +
> > +		led3 {
> > +			label = "blue:user3";
> > +			type = <PCA9532_TYPE_LED>;
> > +		};
> > +	};
> > +};
> > +
> > +&snvs_pwrkey {
> > +	status = "okay";
> > +};
> > +
> > +/* debug console */
> > +&uart2 {
> > +	pinctrl-names = "default";
> > +	pinctrl-0 = <&pinctrl_uart2>;
> > +	status = "okay";
> > +};
> > +
> > +/* SD-Card */
> > +&usdhc2 {
> > +	pinctrl-names = "default", "state_100mhz", "state_200mhz";
> > +	pinctrl-0 = <&pinctrl_usdhc2>, <&pinctrl_usdhc2_pins>;
> > +	pinctrl-1 = <&pinctrl_usdhc2_100mhz>, <&pinctrl_usdhc2_pins>;
> > +	pinctrl-2 = <&pinctrl_usdhc2_200mhz>, <&pinctrl_usdhc2_pins>;
> > +	cd-gpios = <&gpio2 12 GPIO_ACTIVE_LOW>;
> > +	vmmc-supply = <&reg_usdhc2_vmmc>;
> > +	bus-width = <4>;
> > +	status = "okay";
> > +};
> > +
> > +&iomuxc {
> > +	pinctrl_i2c2: i2c2grp {
> > +		fsl,pins = <
> > +			MX8MP_IOMUXC_I2C2_SCL__I2C2_SCL		0x4
> > 00001c3
> > +			MX8MP_IOMUXC_I2C2_SDA__I2C2_SDA		0x4
> > 00001c3
> > +		>;
> > +	};
> > +
> > +	pinctrl_i2c2_gpio: i2c2gpiogrp {
> > +		fsl,pins = <
> > +			MX8MP_IOMUXC_I2C2_SCL__GPIO5_IO16	0x1e3
> > +			MX8MP_IOMUXC_I2C2_SDA__GPIO5_IO17	0x1e3
> > +		>;
> > +	};
> > +
> > +	pinctrl_uart2: uart2grp {
> > +		fsl,pins = <
> > +			MX8MP_IOMUXC_UART2_RXD__UART2_DCE_RX	0x4
> > 9
> > +			MX8MP_IOMUXC_UART2_TXD__UART2_DCE_TX	0x4
> > 9
> > +		>;
> > +	};
> > +
> > +	pinctrl_usdhc2_pins: usdhc2-gpiogrp {
> > +		fsl,pins = <
> > +			MX8MP_IOMUXC_SD2_CD_B__GPIO2_IO12	0x1c4
> > +			MX8MP_IOMUXC_SD2_RESET_B__GPIO2_IO19	0x4
> > 1
> > +		>;
> > +	};
> > +
> > +	pinctrl_usdhc2: usdhc2grp {
> > +		fsl,pins = <
> > +			MX8MP_IOMUXC_SD2_CLK__USDHC2_CLK	0x190
> > +			MX8MP_IOMUXC_SD2_CMD__USDHC2_CMD	0x1d0
> > +			MX8MP_IOMUXC_SD2_DATA0__USDHC2_DATA0	0x1
> > d0
> > +			MX8MP_IOMUXC_SD2_DATA1__USDHC2_DATA1	0x1
> > d0
> > +			MX8MP_IOMUXC_SD2_DATA2__USDHC2_DATA2	0x1
> > d0
> > +			MX8MP_IOMUXC_SD2_DATA3__USDHC2_DATA3	0x1
> > d0
> > +			MX8MP_IOMUXC_GPIO1_IO04__USDHC2_VSELECT	0xc
> > 1
> > +		>;
> > +	};
> > +
> > +	pinctrl_usdhc2_100mhz: usdhc2-100mhzgrp {
> > +		fsl,pins = <
> > +			MX8MP_IOMUXC_SD2_CLK__USDHC2_CLK	0x194
> > +			MX8MP_IOMUXC_SD2_CMD__USDHC2_CMD	0x1d4
> > +			MX8MP_IOMUXC_SD2_DATA0__USDHC2_DATA0	0x1
> > d4
> > +			MX8MP_IOMUXC_SD2_DATA1__USDHC2_DATA1	0x1
> > d4
> > +			MX8MP_IOMUXC_SD2_DATA2__USDHC2_DATA2	0x1
> > d4
> > +			MX8MP_IOMUXC_SD2_DATA3__USDHC2_DATA3	0x1
> > d4
> > +			MX8MP_IOMUXC_GPIO1_IO04__USDHC2_VSELECT	0xc
> > 1
> > +		>;
> > +	};
> > +
> > +	pinctrl_usdhc2_200mhz: usdhc2-200mhzgrp {
> > +		fsl,pins = <
> > +			MX8MP_IOMUXC_SD2_CLK__USDHC2_CLK	0x196
> > +			MX8MP_IOMUXC_SD2_CMD__USDHC2_CMD	0x1d6
> > +			MX8MP_IOMUXC_SD2_DATA0__USDHC2_DATA0	0x1
> > d6
> > +			MX8MP_IOMUXC_SD2_DATA1__USDHC2_DATA1	0x1
> > d6
> > +			MX8MP_IOMUXC_SD2_DATA2__USDHC2_DATA2	0x1
> > d6
> > +			MX8MP_IOMUXC_SD2_DATA3__USDHC2_DATA3	0x1
> > d6
> > +			MX8MP_IOMUXC_GPIO1_IO04__USDHC2_VSELECT	0xc
> > 1
> > +		>;
> > +	};
> > +};
> > diff --git a/arch/arm64/boot/dts/freescale/imx8mp-phycore-som.dtsi
> > b/arch/arm64/boot/dts/freescale/imx8mp-phycore-som.dtsi
> > new file mode 100644
> > index 000000000000..e1fdfebd8142
> > --- /dev/null
> > +++ b/arch/arm64/boot/dts/freescale/imx8mp-phycore-som.dtsi
> > @@ -0,0 +1,319 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Copyright (C) 2020 PHYTEC Messtechnik GmbH
> > + * Author: Teresa Remmet <t.remmet@phytec.de>
> > + */
> > +
> > +#include <dt-bindings/net/ti-dp83867.h>
> > +#include "imx8mp.dtsi"
> > +
> > +/ {
> > +	model = "PHYTEC phyCORE-i.MX8MP";
> > +	compatible = "phytec,imx8mp-phycore-som", "fsl,imx8mp";
> > +
> > +	aliases {
> > +		rtc0 = &rv3028;
> > +		rtc1 = &snvs_rtc;
> > +	};
> > +
> > +	memory@40000000 {
> > +		device_type = "memory";
> > +		reg = <0x0 0x40000000 0 0x80000000>;
> > +	};
> > +
> > +	rtcclkout: rv3028-clkout {
> 
> Is it really a separate oscillator giving 32 kHz? Or maybe this is
> actually part of PMIC?

It is a clock out of the used i2c rtc. Which I actually trying to
disable. As it is not connected. But it is enabled as default.

> 
> In any case node name should reflect class of a device so just
> "clock".

Changing that.

> 
> > +		compatible = "fixed-clock";
> > +		#clock-cells = <0>;
> > +		clock-frequency = <32768>;
> > +		clock-output-names = "rv3028-clkout";
> > +		status = "disabled";
> > +	};
> > +};
> > +
> > +&A53_0 {
> > +	cpu-supply = <&buck2>;
> > +};
> > +
> > +&A53_1 {
> > +	cpu-supply = <&buck2>;
> > +};
> > +
> > +&A53_2 {
> > +	cpu-supply = <&buck2>;
> > +};
> > +
> > +&A53_3 {
> > +	cpu-supply = <&buck2>;
> > +};
> > +
> > +/* ethernet 1 */
> > +&fec {
> > +	pinctrl-names = "default";
> > +	pinctrl-0 = <&pinctrl_fec>;
> > +	phy-mode = "rgmii-id";
> > +	phy-handle = <&ethphy1>;
> > +	fsl,magic-packet;
> > +	status = "okay";
> > +
> > +	mdio {
> > +		#address-cells = <1>;
> > +		#size-cells = <0>;
> > +
> > +		ethphy1: ethernet-phy@0 {
> > +			compatible = "ethernet-phy-ieee802.3-c22";
> > +			reg = <0>;
> > +			interrupt-parent = <&gpio1>;
> > +			interrupts = <15 IRQ_TYPE_EDGE_FALLING>;
> > +			ti,rx-internal-delay =
> > <DP83867_RGMIIDCTL_2_00_NS>;
> > +			ti,tx-internal-delay =
> > <DP83867_RGMIIDCTL_2_00_NS>;
> > +			ti,fifo-depth =
> > <DP83867_PHYCR_FIFO_DEPTH_4_B_NIB>;
> > +			ti,clk-output-sel = <DP83867_CLK_O_SEL_OFF>;
> > +			enet-phy-lane-no-swap;
> > +		};
> > +	};
> > +};
> > +
> > +&i2c1 {
> > +	clock-frequency = <400000>;
> > +	pinctrl-names = "default";
> > +	pinctrl-0 = <&pinctrl_i2c1>;
> > +	pinctrl-1 = <&pinctrl_i2c1_gpio>;
> > +	sda-gpios = <&gpio5 15 (GPIO_ACTIVE_HIGH | GPIO_OPEN_DRAIN)>;
> > +	scl-gpios = <&gpio5 14 (GPIO_ACTIVE_HIGH | GPIO_OPEN_DRAIN)>;
> > +	status = "okay";
> > +
> > +	pmic: pca9450@25 {
> 
> Node name: pmic.

Will be fixed.

> 
> > +		reg = <0x25>;
> > +		compatible = "nxp,pca9450c";
> > +		pinctrl-names = "default";
> > +		pinctrl-0 = <&pinctrl_pmic>;
> > +		interrupt-parent = <&gpio4>;
> > +		interrupts = <18 IRQ_TYPE_LEVEL_LOW>;
> > +
> > +		regulators {
> > +			#address-cells = <1>;
> > +			#size-cells = <0>;
> > +
> > +			buck1: BUCK1 {
> > +				reg = <0>;
> 
> Does this all pass dtc W=1 builds and dtschema? The node name does
> not
> have unit address. Bindings explicitly do not allow it... This looks
> wrong.

The reg property is a leftover and not correct. I will remove this and
others below. Same with #address-cells and #size-cells.

> 
> > +				regulator-compatible = "BUCK1";
> > +				regulator-min-microvolt = <600000>;
> > +				regulator-max-microvolt = <2187500>;
> > +				regulator-boot-on;
> > +				regulator-always-on;
> > +				regulator-ramp-delay = <3125>;
> > +			};
> > +
> > +			buck2: BUCK2 {
> > +				reg = <1>;
> > +				regulator-compatible = "BUCK2";
> > +				regulator-min-microvolt = <600000>;
> > +				regulator-max-microvolt = <2187500>;
> > +				regulator-boot-on;
> > +				regulator-always-on;
> > +				regulator-ramp-delay = <3125>;
> > +			};
> > +
> > +			buck4: BUCK4 {
> > +				reg = <3>;
> > +				regulator-compatible = "BUCK4";
> > +				regulator-min-microvolt = <600000>;
> > +				regulator-max-microvolt = <3400000>;
> > +				regulator-boot-on;
> > +				regulator-always-on;
> > +			};
> > +
> > +			buck5: BUCK5 {
> > +				reg = <4>;
> > +				regulator-compatible = "BUCK5";
> > +				regulator-min-microvolt = <600000>;
> > +				regulator-max-microvolt = <3400000>;
> > +				regulator-boot-on;
> > +				regulator-always-on;
> > +			};
> > +
> > +			buck6: BUCK6 {
> > +				reg = <5>;
> > +				regulator-compatible = "BUCK6";
> > +				regulator-min-microvolt = <600000>;
> > +				regulator-max-microvolt = <3400000>;
> > +				regulator-boot-on;
> > +				regulator-always-on;
> > +			};
> > +
> > +			ldo1: LDO1 {
> > +				reg = <6>;
> > +				regulator-compatible = "LDO1";
> > +				regulator-min-microvolt = <1600000>;
> > +				regulator-max-microvolt = <3300000>;
> > +				regulator-boot-on;
> > +				regulator-always-on;
> > +			};
> > +
> > +			ldo2: LDO2 {
> > +				reg = <7>;
> > +				regulator-compatible = "LDO2";
> > +				regulator-min-microvolt = <800000>;
> > +				regulator-max-microvolt = <1150000>;
> > +				regulator-boot-on;
> > +				regulator-always-on;
> > +			};
> > +
> > +			ldo3: LDO3 {
> > +				reg = <8>;
> > +				regulator-compatible = "LDO3";
> > +				regulator-min-microvolt = <800000>;
> > +				regulator-max-microvolt = <3300000>;
> > +				regulator-boot-on;
> > +				regulator-always-on;
> > +			};
> > +
> > +			ldo4: LDO4 {
> > +				reg = <9>;
> > +				regulator-compatible = "LDO4";
> > +				regulator-min-microvolt = <800000>;
> > +				regulator-max-microvolt = <3300000>;
> > +				regulator-boot-on;
> > +				regulator-always-on;
> > +			};
> > +
> > +			ldo5: LDO5 {
> > +				reg = <10>;
> > +				regulator-compatible = "LDO5";
> > +				regulator-min-microvolt = <1800000>;
> > +				regulator-max-microvolt = <3300000>;
> > +			};
> > +		};
> > +	};
> > +
> > +	eeprom@51 {
> > +		compatible = "atmel,24c32";
> > +		reg = <0x51>;
> > +		pagesize = <32>;
> > +		status = "okay";
> > +	};
> > +
> > +	rv3028: rtc@52 {
> > +		compatible = "microcrystal,rv3028";
> > +		reg = <0x52>;
> > +		trickle-resistor-ohms = <1000>;
> > +		enable-level-switching-mode;
> > +		clocks = <&rtcclkout>;
> > +		status = "okay";
> > +	};
> > +};
> > +
> > +/* eMMC */
> > +&usdhc3 {
> > +	pinctrl-names = "default", "state_100mhz", "state_200mhz";
> > +	pinctrl-0 = <&pinctrl_usdhc3>;
> > +	pinctrl-1 = <&pinctrl_usdhc3_100mhz>;
> > +	pinctrl-2 = <&pinctrl_usdhc3_200mhz>;
> > +	bus-width = <8>;
> > +	non-removable;
> > +	status = "okay";
> > +};
> > +
> > +&wdog1 {
> > +	pinctrl-names = "default";
> > +	pinctrl-0 = <&pinctrl_wdog>;
> > +	fsl,ext-reset-output;
> > +	status = "okay";
> > +};
> > +
> > +&iomuxc {
> > +	pinctrl_fec: fecgrp {
> > +		fsl,pins = <
> > +			MX8MP_IOMUXC_SAI1_RXD2__ENET1_MDC		0x3
> > +			MX8MP_IOMUXC_SAI1_RXD3__ENET1_MDIO		0x3
> > +			MX8MP_IOMUXC_SAI1_RXD4__ENET1_RGMII_RD0		
> > 0x91
> > +			MX8MP_IOMUXC_SAI1_RXD5__ENET1_RGMII_RD1		
> > 0x91
> > +			MX8MP_IOMUXC_SAI1_RXD6__ENET1_RGMII_RD2		
> > 0x91
> > +			MX8MP_IOMUXC_SAI1_RXD7__ENET1_RGMII_RD3		
> > 0x91
> > +			MX8MP_IOMUXC_SAI1_TXC__ENET1_RGMII_RXC		
> > 0x91
> > +			MX8MP_IOMUXC_SAI1_TXFS__ENET1_RGMII_RX_CTL	0x9
> > 1
> > +			MX8MP_IOMUXC_SAI1_TXD0__ENET1_RGMII_TD0		
> > 0x1f
> > +			MX8MP_IOMUXC_SAI1_TXD1__ENET1_RGMII_TD1		
> > 0x1f
> > +			MX8MP_IOMUXC_SAI1_TXD2__ENET1_RGMII_TD2		
> > 0x1f
> > +			MX8MP_IOMUXC_SAI1_TXD3__ENET1_RGMII_TD3		
> > 0x1f
> > +			MX8MP_IOMUXC_SAI1_TXD4__ENET1_RGMII_TX_CTL	0x1
> > f
> > +			MX8MP_IOMUXC_SAI1_TXD5__ENET1_RGMII_TXC		
> > 0x1f
> > +			MX8MP_IOMUXC_GPIO1_IO15__GPIO1_IO15		0x1
> > 1
> > +		>;
> > +	};
> > +
> > +	pinctrl_i2c1: i2c1grp {
> > +		fsl,pins = <
> > +			MX8MP_IOMUXC_I2C1_SCL__I2C1_SCL		0x4
> > 00001c3
> > +			MX8MP_IOMUXC_I2C1_SDA__I2C1_SDA		0x4
> > 00001c3
> > +		>;
> > +	};
> > +
> > +	pinctrl_i2c1_gpio: i2c1gpiogrp {
> > +		fsl,pins = <
> > +			MX8MP_IOMUXC_I2C1_SCL__GPIO5_IO14	0x1e3
> > +			MX8MP_IOMUXC_I2C1_SDA__GPIO5_IO15	0x1e3
> > +		>;
> > +	};
> > +
> > +
> 
> Too many line breaks.

Oh, I will remove this.

Thank you for the review.

Teresa
> 
> Best regards,
> Krzysztof


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 4/4] arm64: dts: freescale: Add support for phyBOARD-Pollux-i.MX8MP

[Krzysztof Kozlowski]

On Mon, Dec 07, 2020 at 02:35:33PM +0100, Teresa Remmet wrote:
> Hello Krzysztof,
> 
> Am Montag, den 07.12.2020, 13:09 +0100 schrieb Krzysztof Kozlowski:
> > On Fri, Dec 04, 2020 at 09:33:02PM +0100, Teresa Remmet wrote:
> > > Add initial support for phyBOARD-Pollux-i.MX8MP.
> > > Supported basic features:
> > > 	* eMMC
> > > 	* i2c EEPROM
> > > 	* i2c RTC
> > > 	* i2c LED
> > > 	* PMIC
> > > 	* debug UART
> > > 	* SD card
> > > 	* 1Gbit Ethernet (fec)
> > > 	* watchdog
> > > 
> > > Signed-off-by: Teresa Remmet <t.remmet@phytec.de>
> > > ---
> > >  arch/arm64/boot/dts/freescale/Makefile             |   1 +
> > >  .../dts/freescale/imx8mp-phyboard-pollux-rdk.dts   |  16 ++
> > >  .../boot/dts/freescale/imx8mp-phyboard-pollux.dtsi | 152
> > > ++++++++++
> > >  .../boot/dts/freescale/imx8mp-phycore-som.dtsi     | 319
> > > +++++++++++++++++++++
> > >  4 files changed, 488 insertions(+)
> > >  create mode 100644 arch/arm64/boot/dts/freescale/imx8mp-phyboard-
> > > pollux-rdk.dts
> > >  create mode 100644 arch/arm64/boot/dts/freescale/imx8mp-phyboard-
> > > pollux.dtsi
> > >  create mode 100644 arch/arm64/boot/dts/freescale/imx8mp-phycore-
> > > som.dtsi
> > > 
> > > diff --git a/arch/arm64/boot/dts/freescale/Makefile
> > > b/arch/arm64/boot/dts/freescale/Makefile
> > > index acfb8af45912..a43b496678be 100644
> > > --- a/arch/arm64/boot/dts/freescale/Makefile
> > > +++ b/arch/arm64/boot/dts/freescale/Makefile
> > > @@ -37,6 +37,7 @@ dtb-$(CONFIG_ARCH_MXC) += imx8mn-evk.dtb
> > >  dtb-$(CONFIG_ARCH_MXC) += imx8mn-ddr4-evk.dtb
> > >  dtb-$(CONFIG_ARCH_MXC) += imx8mn-var-som-symphony.dtb
> > >  dtb-$(CONFIG_ARCH_MXC) += imx8mp-evk.dtb
> > > +dtb-$(CONFIG_ARCH_MXC) += imx8mp-phyboard-pollux-rdk.dtb
> > >  dtb-$(CONFIG_ARCH_MXC) += imx8mq-evk.dtb
> > >  dtb-$(CONFIG_ARCH_MXC) += imx8mq-hummingboard-pulse.dtb
> > >  dtb-$(CONFIG_ARCH_MXC) += imx8mq-librem5-devkit.dtb
> > > diff --git a/arch/arm64/boot/dts/freescale/imx8mp-phyboard-pollux-
> > > rdk.dts b/arch/arm64/boot/dts/freescale/imx8mp-phyboard-pollux-
> > > rdk.dts
> > > new file mode 100644
> > > index 000000000000..dd64be32c99d
> > > --- /dev/null
> > > +++ b/arch/arm64/boot/dts/freescale/imx8mp-phyboard-pollux-rdk.dts
> > > @@ -0,0 +1,16 @@
> > > +// SPDX-License-Identifier: GPL-2.0
> > > +/*
> > > + * Copyright (C) 2020 PHYTEC Messtechnik GmbH
> > > + * Author: Teresa Remmet <t.remmet@phytec.de>
> > > + */
> > > +
> > > +/dts-v1/;
> > > +
> > > +#include "imx8mp-phycore-som.dtsi"
> > > +#include "imx8mp-phyboard-pollux.dtsi"
> > > +
> > > +/ {
> > > +	model = "PHYTEC phyBOARD-Pollux i.MX8MP";
> > > +	compatible = "phytec,imx8mp-phyboard-pollux-rdk",
> > > +		     "phytec,imx8mp-phycore-som", "fsl,imx8mp";
> > 
> > This is the purpose of this file? Why having a DTS to include DTSI
> > only?
> > Usually there is just DTSI for SOM and DTS fot the board.
> 
> we have different options for the SoMs. Like SPI-NOR flash mounted or
> not. We usually add this to the SoM include, but disable it. We enable
> this in the dts if mounted. This makes it easy to generate different
> device trees for different SoM options. So far upstream is not every
> feature supported. So we don't do anything in the dts yet. But I want
> to setup the layout already.
> 
> I hope this makes it clear.

You make the upstream DTSes more complicated to make it easier for
downstream. No, this does not work this way. You can either upstream
other DTSes so such split will make sense, or this contribution should
make sense in the upstreamed state.

In the second case, by "matching upstreamed state" I mean that you
organize your DTSes in a way they make sense for upstream, for example
one DTSI for the SOM and one DTS for the board using it.

> 
> > 
> > > +};
> > > diff --git a/arch/arm64/boot/dts/freescale/imx8mp-phyboard-
> > > pollux.dtsi b/arch/arm64/boot/dts/freescale/imx8mp-phyboard-
> > > pollux.dtsi
> > > new file mode 100644
> > > index 000000000000..dbeaa27eb043
> > > --- /dev/null
> > > +++ b/arch/arm64/boot/dts/freescale/imx8mp-phyboard-pollux.dtsi
> > > @@ -0,0 +1,152 @@
> > > +// SPDX-License-Identifier: GPL-2.0
> > > +/*
> > > + * Copyright (C) 2020 PHYTEC Messtechnik GmbH
> > > + * Author: Teresa Remmet <t.remmet@phytec.de>
> > > + */
> > > +
> > > +#include <dt-bindings/leds/leds-pca9532.h>
> > > +#include <dt-bindings/pwm/pwm.h>
> > > +
> > > +/ {
> > > +	chosen {
> > > +		stdout-path = &uart2;
> > > +	};
> > > +
> > > +	reg_usdhc2_vmmc: regulator-usdhc2 {
> > > +		compatible = "regulator-fixed";
> > > +		regulator-name = "VSD_3V3";
> > > +		regulator-min-microvolt = <3300000>;
> > > +		regulator-max-microvolt = <3300000>;
> > > +		gpio = <&gpio2 19 GPIO_ACTIVE_HIGH>;
> > > +		enable-active-high;
> > > +		startup-delay-us = <100>;
> > > +		off-on-delay-us = <12000>;
> > > +	};
> > > +};
> > > +
> > > +&i2c2 {
> > > +	clock-frequency = <400000>;
> > > +	pinctrl-names = "default";
> > > +	pinctrl-0 = <&pinctrl_i2c2>;
> > > +	pinctrl-1 = <&pinctrl_i2c2_gpio>;
> > > +	sda-gpios = <&gpio5 17 (GPIO_ACTIVE_HIGH | GPIO_OPEN_DRAIN)>;
> > > +	scl-gpios = <&gpio5 16 (GPIO_ACTIVE_HIGH | GPIO_OPEN_DRAIN)>;
> > > +	status = "okay";
> > > +
> > > +	eeprom@51 {
> > > +		compatible = "atmel,24c02";
> > > +		reg = <0x51>;
> > > +		pagesize = <16>;
> > > +		status = "okay";
> > > +	};
> > > +
> > > +	leddimmer@62 {
> > 
> > I think name "leds" is more appropriate.
> 
> okay I will change it.
> 
> > 
> > > +		compatible = "nxp,pca9533";
> > > +		reg = <0x62>;
> > > +		status = "okay";
> > > +
> > > +		led1 {
> > > +			label = "red:user1";
> > > +			type = <PCA9532_TYPE_LED>;
> > > +		};
> > > +
> > > +		led2 {
> > > +			label = "green:user2";
> > > +			type = <PCA9532_TYPE_LED>;
> > > +		};
> > > +
> > > +		led3 {
> > > +			label = "blue:user3";
> > > +			type = <PCA9532_TYPE_LED>;
> > > +		};
> > > +	};
> > > +};
> > > +
> > > +&snvs_pwrkey {
> > > +	status = "okay";
> > > +};
> > > +
> > > +/* debug console */
> > > +&uart2 {
> > > +	pinctrl-names = "default";
> > > +	pinctrl-0 = <&pinctrl_uart2>;
> > > +	status = "okay";
> > > +};
> > > +
> > > +/* SD-Card */
> > > +&usdhc2 {
> > > +	pinctrl-names = "default", "state_100mhz", "state_200mhz";
> > > +	pinctrl-0 = <&pinctrl_usdhc2>, <&pinctrl_usdhc2_pins>;
> > > +	pinctrl-1 = <&pinctrl_usdhc2_100mhz>, <&pinctrl_usdhc2_pins>;
> > > +	pinctrl-2 = <&pinctrl_usdhc2_200mhz>, <&pinctrl_usdhc2_pins>;
> > > +	cd-gpios = <&gpio2 12 GPIO_ACTIVE_LOW>;
> > > +	vmmc-supply = <&reg_usdhc2_vmmc>;
> > > +	bus-width = <4>;
> > > +	status = "okay";
> > > +};
> > > +
> > > +&iomuxc {
> > > +	pinctrl_i2c2: i2c2grp {
> > > +		fsl,pins = <
> > > +			MX8MP_IOMUXC_I2C2_SCL__I2C2_SCL		0x4
> > > 00001c3
> > > +			MX8MP_IOMUXC_I2C2_SDA__I2C2_SDA		0x4
> > > 00001c3
> > > +		>;
> > > +	};
> > > +
> > > +	pinctrl_i2c2_gpio: i2c2gpiogrp {
> > > +		fsl,pins = <
> > > +			MX8MP_IOMUXC_I2C2_SCL__GPIO5_IO16	0x1e3
> > > +			MX8MP_IOMUXC_I2C2_SDA__GPIO5_IO17	0x1e3
> > > +		>;
> > > +	};
> > > +
> > > +	pinctrl_uart2: uart2grp {
> > > +		fsl,pins = <
> > > +			MX8MP_IOMUXC_UART2_RXD__UART2_DCE_RX	0x4
> > > 9
> > > +			MX8MP_IOMUXC_UART2_TXD__UART2_DCE_TX	0x4
> > > 9
> > > +		>;
> > > +	};
> > > +
> > > +	pinctrl_usdhc2_pins: usdhc2-gpiogrp {
> > > +		fsl,pins = <
> > > +			MX8MP_IOMUXC_SD2_CD_B__GPIO2_IO12	0x1c4
> > > +			MX8MP_IOMUXC_SD2_RESET_B__GPIO2_IO19	0x4
> > > 1
> > > +		>;
> > > +	};
> > > +
> > > +	pinctrl_usdhc2: usdhc2grp {
> > > +		fsl,pins = <
> > > +			MX8MP_IOMUXC_SD2_CLK__USDHC2_CLK	0x190
> > > +			MX8MP_IOMUXC_SD2_CMD__USDHC2_CMD	0x1d0
> > > +			MX8MP_IOMUXC_SD2_DATA0__USDHC2_DATA0	0x1
> > > d0
> > > +			MX8MP_IOMUXC_SD2_DATA1__USDHC2_DATA1	0x1
> > > d0
> > > +			MX8MP_IOMUXC_SD2_DATA2__USDHC2_DATA2	0x1
> > > d0
> > > +			MX8MP_IOMUXC_SD2_DATA3__USDHC2_DATA3	0x1
> > > d0
> > > +			MX8MP_IOMUXC_GPIO1_IO04__USDHC2_VSELECT	0xc
> > > 1
> > > +		>;
> > > +	};
> > > +
> > > +	pinctrl_usdhc2_100mhz: usdhc2-100mhzgrp {
> > > +		fsl,pins = <
> > > +			MX8MP_IOMUXC_SD2_CLK__USDHC2_CLK	0x194
> > > +			MX8MP_IOMUXC_SD2_CMD__USDHC2_CMD	0x1d4
> > > +			MX8MP_IOMUXC_SD2_DATA0__USDHC2_DATA0	0x1
> > > d4
> > > +			MX8MP_IOMUXC_SD2_DATA1__USDHC2_DATA1	0x1
> > > d4
> > > +			MX8MP_IOMUXC_SD2_DATA2__USDHC2_DATA2	0x1
> > > d4
> > > +			MX8MP_IOMUXC_SD2_DATA3__USDHC2_DATA3	0x1
> > > d4
> > > +			MX8MP_IOMUXC_GPIO1_IO04__USDHC2_VSELECT	0xc
> > > 1
> > > +		>;
> > > +	};
> > > +
> > > +	pinctrl_usdhc2_200mhz: usdhc2-200mhzgrp {
> > > +		fsl,pins = <
> > > +			MX8MP_IOMUXC_SD2_CLK__USDHC2_CLK	0x196
> > > +			MX8MP_IOMUXC_SD2_CMD__USDHC2_CMD	0x1d6
> > > +			MX8MP_IOMUXC_SD2_DATA0__USDHC2_DATA0	0x1
> > > d6
> > > +			MX8MP_IOMUXC_SD2_DATA1__USDHC2_DATA1	0x1
> > > d6
> > > +			MX8MP_IOMUXC_SD2_DATA2__USDHC2_DATA2	0x1
> > > d6
> > > +			MX8MP_IOMUXC_SD2_DATA3__USDHC2_DATA3	0x1
> > > d6
> > > +			MX8MP_IOMUXC_GPIO1_IO04__USDHC2_VSELECT	0xc
> > > 1
> > > +		>;
> > > +	};
> > > +};
> > > diff --git a/arch/arm64/boot/dts/freescale/imx8mp-phycore-som.dtsi
> > > b/arch/arm64/boot/dts/freescale/imx8mp-phycore-som.dtsi
> > > new file mode 100644
> > > index 000000000000..e1fdfebd8142
> > > --- /dev/null
> > > +++ b/arch/arm64/boot/dts/freescale/imx8mp-phycore-som.dtsi
> > > @@ -0,0 +1,319 @@
> > > +// SPDX-License-Identifier: GPL-2.0
> > > +/*
> > > + * Copyright (C) 2020 PHYTEC Messtechnik GmbH
> > > + * Author: Teresa Remmet <t.remmet@phytec.de>
> > > + */
> > > +
> > > +#include <dt-bindings/net/ti-dp83867.h>
> > > +#include "imx8mp.dtsi"
> > > +
> > > +/ {
> > > +	model = "PHYTEC phyCORE-i.MX8MP";
> > > +	compatible = "phytec,imx8mp-phycore-som", "fsl,imx8mp";
> > > +
> > > +	aliases {
> > > +		rtc0 = &rv3028;
> > > +		rtc1 = &snvs_rtc;
> > > +	};
> > > +
> > > +	memory@40000000 {
> > > +		device_type = "memory";
> > > +		reg = <0x0 0x40000000 0 0x80000000>;
> > > +	};
> > > +
> > > +	rtcclkout: rv3028-clkout {
> > 
> > Is it really a separate oscillator giving 32 kHz? Or maybe this is
> > actually part of PMIC?
> 
> It is a clock out of the used i2c rtc. Which I actually trying to
> disable. As it is not connected. But it is enabled as default.

This does not make sense at all:
1. This is a node without any reference to hardware,
2. It is being disabled in DTS so it will not have any effect in kernel
   therefore will not have any impact on real hardware,
3. The RV3028 RTC takes it as phandle... but RV3028 RTC is a clock
   provider, not a consumer.

If you want to disable RV3028 RTC clock, you need to modify it's
properties, not add fake nodes.

Best regards,
Krzysztof

Re: [PATCH 4/4] arm64: dts: freescale: Add support for phyBOARD-Pollux-i.MX8MP

[Krzysztof Kozlowski]

On Mon, Dec 07, 2020 at 02:35:33PM +0100, Teresa Remmet wrote:
> Hello Krzysztof,
> 
> Am Montag, den 07.12.2020, 13:09 +0100 schrieb Krzysztof Kozlowski:
> > On Fri, Dec 04, 2020 at 09:33:02PM +0100, Teresa Remmet wrote:
> > > Add initial support for phyBOARD-Pollux-i.MX8MP.
> > > Supported basic features:
> > > 	* eMMC
> > > 	* i2c EEPROM
> > > 	* i2c RTC
> > > 	* i2c LED
> > > 	* PMIC
> > > 	* debug UART
> > > 	* SD card
> > > 	* 1Gbit Ethernet (fec)
> > > 	* watchdog
> > > 
> > > Signed-off-by: Teresa Remmet <t.remmet@phytec.de>
> > > ---
> > >  arch/arm64/boot/dts/freescale/Makefile             |   1 +
> > >  .../dts/freescale/imx8mp-phyboard-pollux-rdk.dts   |  16 ++
> > >  .../boot/dts/freescale/imx8mp-phyboard-pollux.dtsi | 152
> > > ++++++++++
> > >  .../boot/dts/freescale/imx8mp-phycore-som.dtsi     | 319
> > > +++++++++++++++++++++
> > >  4 files changed, 488 insertions(+)
> > >  create mode 100644 arch/arm64/boot/dts/freescale/imx8mp-phyboard-
> > > pollux-rdk.dts
> > >  create mode 100644 arch/arm64/boot/dts/freescale/imx8mp-phyboard-
> > > pollux.dtsi
> > >  create mode 100644 arch/arm64/boot/dts/freescale/imx8mp-phycore-
> > > som.dtsi
> > > 
> > > diff --git a/arch/arm64/boot/dts/freescale/Makefile
> > > b/arch/arm64/boot/dts/freescale/Makefile
> > > index acfb8af45912..a43b496678be 100644
> > > --- a/arch/arm64/boot/dts/freescale/Makefile
> > > +++ b/arch/arm64/boot/dts/freescale/Makefile
> > > @@ -37,6 +37,7 @@ dtb-$(CONFIG_ARCH_MXC) += imx8mn-evk.dtb
> > >  dtb-$(CONFIG_ARCH_MXC) += imx8mn-ddr4-evk.dtb
> > >  dtb-$(CONFIG_ARCH_MXC) += imx8mn-var-som-symphony.dtb
> > >  dtb-$(CONFIG_ARCH_MXC) += imx8mp-evk.dtb
> > > +dtb-$(CONFIG_ARCH_MXC) += imx8mp-phyboard-pollux-rdk.dtb
> > >  dtb-$(CONFIG_ARCH_MXC) += imx8mq-evk.dtb
> > >  dtb-$(CONFIG_ARCH_MXC) += imx8mq-hummingboard-pulse.dtb
> > >  dtb-$(CONFIG_ARCH_MXC) += imx8mq-librem5-devkit.dtb
> > > diff --git a/arch/arm64/boot/dts/freescale/imx8mp-phyboard-pollux-
> > > rdk.dts b/arch/arm64/boot/dts/freescale/imx8mp-phyboard-pollux-
> > > rdk.dts
> > > new file mode 100644
> > > index 000000000000..dd64be32c99d
> > > --- /dev/null
> > > +++ b/arch/arm64/boot/dts/freescale/imx8mp-phyboard-pollux-rdk.dts
> > > @@ -0,0 +1,16 @@
> > > +// SPDX-License-Identifier: GPL-2.0
> > > +/*
> > > + * Copyright (C) 2020 PHYTEC Messtechnik GmbH
> > > + * Author: Teresa Remmet <t.remmet@phytec.de>
> > > + */
> > > +
> > > +/dts-v1/;
> > > +
> > > +#include "imx8mp-phycore-som.dtsi"
> > > +#include "imx8mp-phyboard-pollux.dtsi"
> > > +
> > > +/ {
> > > +	model = "PHYTEC phyBOARD-Pollux i.MX8MP";
> > > +	compatible = "phytec,imx8mp-phyboard-pollux-rdk",
> > > +		     "phytec,imx8mp-phycore-som", "fsl,imx8mp";
> > 
> > This is the purpose of this file? Why having a DTS to include DTSI
> > only?
> > Usually there is just DTSI for SOM and DTS fot the board.
> 
> we have different options for the SoMs. Like SPI-NOR flash mounted or
> not. We usually add this to the SoM include, but disable it. We enable
> this in the dts if mounted. This makes it easy to generate different
> device trees for different SoM options. So far upstream is not every
> feature supported. So we don't do anything in the dts yet. But I want
> to setup the layout already.
> 
> I hope this makes it clear.

You make the upstream DTSes more complicated to make it easier for
downstream. No, this does not work this way. You can either upstream
other DTSes so such split will make sense, or this contribution should
make sense in the upstreamed state.

In the second case, by "matching upstreamed state" I mean that you
organize your DTSes in a way they make sense for upstream, for example
one DTSI for the SOM and one DTS for the board using it.

> 
> > 
> > > +};
> > > diff --git a/arch/arm64/boot/dts/freescale/imx8mp-phyboard-
> > > pollux.dtsi b/arch/arm64/boot/dts/freescale/imx8mp-phyboard-
> > > pollux.dtsi
> > > new file mode 100644
> > > index 000000000000..dbeaa27eb043
> > > --- /dev/null
> > > +++ b/arch/arm64/boot/dts/freescale/imx8mp-phyboard-pollux.dtsi
> > > @@ -0,0 +1,152 @@
> > > +// SPDX-License-Identifier: GPL-2.0
> > > +/*
> > > + * Copyright (C) 2020 PHYTEC Messtechnik GmbH
> > > + * Author: Teresa Remmet <t.remmet@phytec.de>
> > > + */
> > > +
> > > +#include <dt-bindings/leds/leds-pca9532.h>
> > > +#include <dt-bindings/pwm/pwm.h>
> > > +
> > > +/ {
> > > +	chosen {
> > > +		stdout-path = &uart2;
> > > +	};
> > > +
> > > +	reg_usdhc2_vmmc: regulator-usdhc2 {
> > > +		compatible = "regulator-fixed";
> > > +		regulator-name = "VSD_3V3";
> > > +		regulator-min-microvolt = <3300000>;
> > > +		regulator-max-microvolt = <3300000>;
> > > +		gpio = <&gpio2 19 GPIO_ACTIVE_HIGH>;
> > > +		enable-active-high;
> > > +		startup-delay-us = <100>;
> > > +		off-on-delay-us = <12000>;
> > > +	};
> > > +};
> > > +
> > > +&i2c2 {
> > > +	clock-frequency = <400000>;
> > > +	pinctrl-names = "default";
> > > +	pinctrl-0 = <&pinctrl_i2c2>;
> > > +	pinctrl-1 = <&pinctrl_i2c2_gpio>;
> > > +	sda-gpios = <&gpio5 17 (GPIO_ACTIVE_HIGH | GPIO_OPEN_DRAIN)>;
> > > +	scl-gpios = <&gpio5 16 (GPIO_ACTIVE_HIGH | GPIO_OPEN_DRAIN)>;
> > > +	status = "okay";
> > > +
> > > +	eeprom@51 {
> > > +		compatible = "atmel,24c02";
> > > +		reg = <0x51>;
> > > +		pagesize = <16>;
> > > +		status = "okay";
> > > +	};
> > > +
> > > +	leddimmer@62 {
> > 
> > I think name "leds" is more appropriate.
> 
> okay I will change it.
> 
> > 
> > > +		compatible = "nxp,pca9533";
> > > +		reg = <0x62>;
> > > +		status = "okay";
> > > +
> > > +		led1 {
> > > +			label = "red:user1";
> > > +			type = <PCA9532_TYPE_LED>;
> > > +		};
> > > +
> > > +		led2 {
> > > +			label = "green:user2";
> > > +			type = <PCA9532_TYPE_LED>;
> > > +		};
> > > +
> > > +		led3 {
> > > +			label = "blue:user3";
> > > +			type = <PCA9532_TYPE_LED>;
> > > +		};
> > > +	};
> > > +};
> > > +
> > > +&snvs_pwrkey {
> > > +	status = "okay";
> > > +};
> > > +
> > > +/* debug console */
> > > +&uart2 {
> > > +	pinctrl-names = "default";
> > > +	pinctrl-0 = <&pinctrl_uart2>;
> > > +	status = "okay";
> > > +};
> > > +
> > > +/* SD-Card */
> > > +&usdhc2 {
> > > +	pinctrl-names = "default", "state_100mhz", "state_200mhz";
> > > +	pinctrl-0 = <&pinctrl_usdhc2>, <&pinctrl_usdhc2_pins>;
> > > +	pinctrl-1 = <&pinctrl_usdhc2_100mhz>, <&pinctrl_usdhc2_pins>;
> > > +	pinctrl-2 = <&pinctrl_usdhc2_200mhz>, <&pinctrl_usdhc2_pins>;
> > > +	cd-gpios = <&gpio2 12 GPIO_ACTIVE_LOW>;
> > > +	vmmc-supply = <&reg_usdhc2_vmmc>;
> > > +	bus-width = <4>;
> > > +	status = "okay";
> > > +};
> > > +
> > > +&iomuxc {
> > > +	pinctrl_i2c2: i2c2grp {
> > > +		fsl,pins = <
> > > +			MX8MP_IOMUXC_I2C2_SCL__I2C2_SCL		0x4
> > > 00001c3
> > > +			MX8MP_IOMUXC_I2C2_SDA__I2C2_SDA		0x4
> > > 00001c3
> > > +		>;
> > > +	};
> > > +
> > > +	pinctrl_i2c2_gpio: i2c2gpiogrp {
> > > +		fsl,pins = <
> > > +			MX8MP_IOMUXC_I2C2_SCL__GPIO5_IO16	0x1e3
> > > +			MX8MP_IOMUXC_I2C2_SDA__GPIO5_IO17	0x1e3
> > > +		>;
> > > +	};
> > > +
> > > +	pinctrl_uart2: uart2grp {
> > > +		fsl,pins = <
> > > +			MX8MP_IOMUXC_UART2_RXD__UART2_DCE_RX	0x4
> > > 9
> > > +			MX8MP_IOMUXC_UART2_TXD__UART2_DCE_TX	0x4
> > > 9
> > > +		>;
> > > +	};
> > > +
> > > +	pinctrl_usdhc2_pins: usdhc2-gpiogrp {
> > > +		fsl,pins = <
> > > +			MX8MP_IOMUXC_SD2_CD_B__GPIO2_IO12	0x1c4
> > > +			MX8MP_IOMUXC_SD2_RESET_B__GPIO2_IO19	0x4
> > > 1
> > > +		>;
> > > +	};
> > > +
> > > +	pinctrl_usdhc2: usdhc2grp {
> > > +		fsl,pins = <
> > > +			MX8MP_IOMUXC_SD2_CLK__USDHC2_CLK	0x190
> > > +			MX8MP_IOMUXC_SD2_CMD__USDHC2_CMD	0x1d0
> > > +			MX8MP_IOMUXC_SD2_DATA0__USDHC2_DATA0	0x1
> > > d0
> > > +			MX8MP_IOMUXC_SD2_DATA1__USDHC2_DATA1	0x1
> > > d0
> > > +			MX8MP_IOMUXC_SD2_DATA2__USDHC2_DATA2	0x1
> > > d0
> > > +			MX8MP_IOMUXC_SD2_DATA3__USDHC2_DATA3	0x1
> > > d0
> > > +			MX8MP_IOMUXC_GPIO1_IO04__USDHC2_VSELECT	0xc
> > > 1
> > > +		>;
> > > +	};
> > > +
> > > +	pinctrl_usdhc2_100mhz: usdhc2-100mhzgrp {
> > > +		fsl,pins = <
> > > +			MX8MP_IOMUXC_SD2_CLK__USDHC2_CLK	0x194
> > > +			MX8MP_IOMUXC_SD2_CMD__USDHC2_CMD	0x1d4
> > > +			MX8MP_IOMUXC_SD2_DATA0__USDHC2_DATA0	0x1
> > > d4
> > > +			MX8MP_IOMUXC_SD2_DATA1__USDHC2_DATA1	0x1
> > > d4
> > > +			MX8MP_IOMUXC_SD2_DATA2__USDHC2_DATA2	0x1
> > > d4
> > > +			MX8MP_IOMUXC_SD2_DATA3__USDHC2_DATA3	0x1
> > > d4
> > > +			MX8MP_IOMUXC_GPIO1_IO04__USDHC2_VSELECT	0xc
> > > 1
> > > +		>;
> > > +	};
> > > +
> > > +	pinctrl_usdhc2_200mhz: usdhc2-200mhzgrp {
> > > +		fsl,pins = <
> > > +			MX8MP_IOMUXC_SD2_CLK__USDHC2_CLK	0x196
> > > +			MX8MP_IOMUXC_SD2_CMD__USDHC2_CMD	0x1d6
> > > +			MX8MP_IOMUXC_SD2_DATA0__USDHC2_DATA0	0x1
> > > d6
> > > +			MX8MP_IOMUXC_SD2_DATA1__USDHC2_DATA1	0x1
> > > d6
> > > +			MX8MP_IOMUXC_SD2_DATA2__USDHC2_DATA2	0x1
> > > d6
> > > +			MX8MP_IOMUXC_SD2_DATA3__USDHC2_DATA3	0x1
> > > d6
> > > +			MX8MP_IOMUXC_GPIO1_IO04__USDHC2_VSELECT	0xc
> > > 1
> > > +		>;
> > > +	};
> > > +};
> > > diff --git a/arch/arm64/boot/dts/freescale/imx8mp-phycore-som.dtsi
> > > b/arch/arm64/boot/dts/freescale/imx8mp-phycore-som.dtsi
> > > new file mode 100644
> > > index 000000000000..e1fdfebd8142
> > > --- /dev/null
> > > +++ b/arch/arm64/boot/dts/freescale/imx8mp-phycore-som.dtsi
> > > @@ -0,0 +1,319 @@
> > > +// SPDX-License-Identifier: GPL-2.0
> > > +/*
> > > + * Copyright (C) 2020 PHYTEC Messtechnik GmbH
> > > + * Author: Teresa Remmet <t.remmet@phytec.de>
> > > + */
> > > +
> > > +#include <dt-bindings/net/ti-dp83867.h>
> > > +#include "imx8mp.dtsi"
> > > +
> > > +/ {
> > > +	model = "PHYTEC phyCORE-i.MX8MP";
> > > +	compatible = "phytec,imx8mp-phycore-som", "fsl,imx8mp";
> > > +
> > > +	aliases {
> > > +		rtc0 = &rv3028;
> > > +		rtc1 = &snvs_rtc;
> > > +	};
> > > +
> > > +	memory@40000000 {
> > > +		device_type = "memory";
> > > +		reg = <0x0 0x40000000 0 0x80000000>;
> > > +	};
> > > +
> > > +	rtcclkout: rv3028-clkout {
> > 
> > Is it really a separate oscillator giving 32 kHz? Or maybe this is
> > actually part of PMIC?
> 
> It is a clock out of the used i2c rtc. Which I actually trying to
> disable. As it is not connected. But it is enabled as default.

This does not make sense at all:
1. This is a node without any reference to hardware,
2. It is being disabled in DTS so it will not have any effect in kernel
   therefore will not have any impact on real hardware,
3. The RV3028 RTC takes it as phandle... but RV3028 RTC is a clock
   provider, not a consumer.

If you want to disable RV3028 RTC clock, you need to modify it's
properties, not add fake nodes.

Best regards,
Krzysztof

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 4/4] arm64: dts: freescale: Add support for phyBOARD-Pollux-i.MX8MP

[Teresa Remmet]

Hello Krzysztof,

Am Montag, den 07.12.2020, 14:46 +0100 schrieb Krzysztof Kozlowski:
> On Mon, Dec 07, 2020 at 02:35:33PM +0100, Teresa Remmet wrote:
> > Hello Krzysztof,
> > 
> > Am Montag, den 07.12.2020, 13:09 +0100 schrieb Krzysztof Kozlowski:
> > > On Fri, Dec 04, 2020 at 09:33:02PM +0100, Teresa Remmet wrote:
> > > > Add initial support for phyBOARD-Pollux-i.MX8MP.
> > > > Supported basic features:
> > > > 	* eMMC
> > > > 	* i2c EEPROM
> > > > 	* i2c RTC
> > > > 	* i2c LED
> > > > 	* PMIC
> > > > 	* debug UART
> > > > 	* SD card
> > > > 	* 1Gbit Ethernet (fec)
> > > > 	* watchdog
> > > > 
> > > > Signed-off-by: Teresa Remmet <t.remmet@phytec.de>
> > > > ---
> > > >  arch/arm64/boot/dts/freescale/Makefile             |   1 +
> > > >  .../dts/freescale/imx8mp-phyboard-pollux-rdk.dts   |  16 ++
> > > >  .../boot/dts/freescale/imx8mp-phyboard-pollux.dtsi | 152
> > > > ++++++++++
> > > >  .../boot/dts/freescale/imx8mp-phycore-som.dtsi     | 319
> > > > +++++++++++++++++++++
> > > >  4 files changed, 488 insertions(+)
> > > >  create mode 100644 arch/arm64/boot/dts/freescale/imx8mp-
> > > > phyboard-
> > > > pollux-rdk.dts
> > > >  create mode 100644 arch/arm64/boot/dts/freescale/imx8mp-
> > > > phyboard-
> > > > pollux.dtsi
> > > >  create mode 100644 arch/arm64/boot/dts/freescale/imx8mp-
> > > > phycore-
> > > > som.dtsi
> > > > 
> > > > diff --git a/arch/arm64/boot/dts/freescale/Makefile
> > > > b/arch/arm64/boot/dts/freescale/Makefile
> > > > index acfb8af45912..a43b496678be 100644
> > > > --- a/arch/arm64/boot/dts/freescale/Makefile
> > > > +++ b/arch/arm64/boot/dts/freescale/Makefile
> > > > @@ -37,6 +37,7 @@ dtb-$(CONFIG_ARCH_MXC) += imx8mn-evk.dtb
> > > >  dtb-$(CONFIG_ARCH_MXC) += imx8mn-ddr4-evk.dtb
> > > >  dtb-$(CONFIG_ARCH_MXC) += imx8mn-var-som-symphony.dtb
> > > >  dtb-$(CONFIG_ARCH_MXC) += imx8mp-evk.dtb
> > > > +dtb-$(CONFIG_ARCH_MXC) += imx8mp-phyboard-pollux-rdk.dtb
> > > >  dtb-$(CONFIG_ARCH_MXC) += imx8mq-evk.dtb
> > > >  dtb-$(CONFIG_ARCH_MXC) += imx8mq-hummingboard-pulse.dtb
> > > >  dtb-$(CONFIG_ARCH_MXC) += imx8mq-librem5-devkit.dtb
> > > > diff --git a/arch/arm64/boot/dts/freescale/imx8mp-phyboard-
> > > > pollux-
> > > > rdk.dts b/arch/arm64/boot/dts/freescale/imx8mp-phyboard-pollux-
> > > > rdk.dts
> > > > new file mode 100644
> > > > index 000000000000..dd64be32c99d
> > > > --- /dev/null
> > > > +++ b/arch/arm64/boot/dts/freescale/imx8mp-phyboard-pollux-
> > > > rdk.dts
> > > > @@ -0,0 +1,16 @@
> > > > +// SPDX-License-Identifier: GPL-2.0
> > > > +/*
> > > > + * Copyright (C) 2020 PHYTEC Messtechnik GmbH
> > > > + * Author: Teresa Remmet <t.remmet@phytec.de>
> > > > + */
> > > > +
> > > > +/dts-v1/;
> > > > +
> > > > +#include "imx8mp-phycore-som.dtsi"
> > > > +#include "imx8mp-phyboard-pollux.dtsi"
> > > > +
> > > > +/ {
> > > > +	model = "PHYTEC phyBOARD-Pollux i.MX8MP";
> > > > +	compatible = "phytec,imx8mp-phyboard-pollux-rdk",
> > > > +		     "phytec,imx8mp-phycore-som", "fsl,imx8mp";
> > > 
> > > This is the purpose of this file? Why having a DTS to include
> > > DTSI
> > > only?
> > > Usually there is just DTSI for SOM and DTS fot the board.
> > 
> > we have different options for the SoMs. Like SPI-NOR flash mounted
> > or
> > not. We usually add this to the SoM include, but disable it. We
> > enable
> > this in the dts if mounted. This makes it easy to generate
> > different
> > device trees for different SoM options. So far upstream is not
> > every
> > feature supported. So we don't do anything in the dts yet. But I
> > want
> > to setup the layout already.
> > 
> > I hope this makes it clear.
> 
> You make the upstream DTSes more complicated to make it easier for
> downstream. No, this does not work this way. You can either upstream
> other DTSes so such split will make sense, or this contribution
> should
> make sense in the upstreamed state.
> 
> In the second case, by "matching upstreamed state" I mean that you
> organize your DTSes in a way they make sense for upstream, for
> example
> one DTSI for the SOM and one DTS for the board using it.

Ok, then i will change it now like you suggested and rework it later
after more features are available.

> 
> > 
> > > 
> > > > +};
> > > > diff --git a/arch/arm64/boot/dts/freescale/imx8mp-phyboard-
> > > > pollux.dtsi b/arch/arm64/boot/dts/freescale/imx8mp-phyboard-
> > > > pollux.dtsi
> > > > new file mode 100644
> > > > index 000000000000..dbeaa27eb043
> > > > --- /dev/null
> > > > +++ b/arch/arm64/boot/dts/freescale/imx8mp-phyboard-pollux.dtsi
> > > > @@ -0,0 +1,152 @@
> > > > +// SPDX-License-Identifier: GPL-2.0
> > > > +/*
> > > > + * Copyright (C) 2020 PHYTEC Messtechnik GmbH
> > > > + * Author: Teresa Remmet <t.remmet@phytec.de>
> > > > + */
> > > > +
> > > > +#include <dt-bindings/leds/leds-pca9532.h>
> > > > +#include <dt-bindings/pwm/pwm.h>
> > > > +
> > > > +/ {
> > > > +	chosen {
> > > > +		stdout-path = &uart2;
> > > > +	};
> > > > +
> > > > +	reg_usdhc2_vmmc: regulator-usdhc2 {
> > > > +		compatible = "regulator-fixed";
> > > > +		regulator-name = "VSD_3V3";
> > > > +		regulator-min-microvolt = <3300000>;
> > > > +		regulator-max-microvolt = <3300000>;
> > > > +		gpio = <&gpio2 19 GPIO_ACTIVE_HIGH>;
> > > > +		enable-active-high;
> > > > +		startup-delay-us = <100>;
> > > > +		off-on-delay-us = <12000>;
> > > > +	};
> > > > +};
> > > > +
> > > > +&i2c2 {
> > > > +	clock-frequency = <400000>;
> > > > +	pinctrl-names = "default";
> > > > +	pinctrl-0 = <&pinctrl_i2c2>;
> > > > +	pinctrl-1 = <&pinctrl_i2c2_gpio>;
> > > > +	sda-gpios = <&gpio5 17 (GPIO_ACTIVE_HIGH |
> > > > GPIO_OPEN_DRAIN)>;
> > > > +	scl-gpios = <&gpio5 16 (GPIO_ACTIVE_HIGH |
> > > > GPIO_OPEN_DRAIN)>;
> > > > +	status = "okay";
> > > > +
> > > > +	eeprom@51 {
> > > > +		compatible = "atmel,24c02";
> > > > +		reg = <0x51>;
> > > > +		pagesize = <16>;
> > > > +		status = "okay";
> > > > +	};
> > > > +
> > > > +	leddimmer@62 {
> > > 
> > > I think name "leds" is more appropriate.
> > 
> > okay I will change it.
> > 
> > > 
> > > > +		compatible = "nxp,pca9533";
> > > > +		reg = <0x62>;
> > > > +		status = "okay";
> > > > +
> > > > +		led1 {
> > > > +			label = "red:user1";
> > > > +			type = <PCA9532_TYPE_LED>;
> > > > +		};
> > > > +
> > > > +		led2 {
> > > > +			label = "green:user2";
> > > > +			type = <PCA9532_TYPE_LED>;
> > > > +		};
> > > > +
> > > > +		led3 {
> > > > +			label = "blue:user3";
> > > > +			type = <PCA9532_TYPE_LED>;
> > > > +		};
> > > > +	};
> > > > +};
> > > > +
> > > > +&snvs_pwrkey {
> > > > +	status = "okay";
> > > > +};
> > > > +
> > > > +/* debug console */
> > > > +&uart2 {
> > > > +	pinctrl-names = "default";
> > > > +	pinctrl-0 = <&pinctrl_uart2>;
> > > > +	status = "okay";
> > > > +};
> > > > +
> > > > +/* SD-Card */
> > > > +&usdhc2 {
> > > > +	pinctrl-names = "default", "state_100mhz",
> > > > "state_200mhz";
> > > > +	pinctrl-0 = <&pinctrl_usdhc2>, <&pinctrl_usdhc2_pins>;
> > > > +	pinctrl-1 = <&pinctrl_usdhc2_100mhz>,
> > > > <&pinctrl_usdhc2_pins>;
> > > > +	pinctrl-2 = <&pinctrl_usdhc2_200mhz>,
> > > > <&pinctrl_usdhc2_pins>;
> > > > +	cd-gpios = <&gpio2 12 GPIO_ACTIVE_LOW>;
> > > > +	vmmc-supply = <&reg_usdhc2_vmmc>;
> > > > +	bus-width = <4>;
> > > > +	status = "okay";
> > > > +};
> > > > +
> > > > +&iomuxc {
> > > > +	pinctrl_i2c2: i2c2grp {
> > > > +		fsl,pins = <
> > > > +			MX8MP_IOMUXC_I2C2_SCL__I2C2_SCL		
> > > > 0x4
> > > > 00001c3
> > > > +			MX8MP_IOMUXC_I2C2_SDA__I2C2_SDA		
> > > > 0x4
> > > > 00001c3
> > > > +		>;
> > > > +	};
> > > > +
> > > > +	pinctrl_i2c2_gpio: i2c2gpiogrp {
> > > > +		fsl,pins = <
> > > > +			MX8MP_IOMUXC_I2C2_SCL__GPIO5_IO16	
> > > > 0x1e3
> > > > +			MX8MP_IOMUXC_I2C2_SDA__GPIO5_IO17	
> > > > 0x1e3
> > > > +		>;
> > > > +	};
> > > > +
> > > > +	pinctrl_uart2: uart2grp {
> > > > +		fsl,pins = <
> > > > +			MX8MP_IOMUXC_UART2_RXD__UART2_DCE_RX	
> > > > 0x4
> > > > 9
> > > > +			MX8MP_IOMUXC_UART2_TXD__UART2_DCE_TX	
> > > > 0x4
> > > > 9
> > > > +		>;
> > > > +	};
> > > > +
> > > > +	pinctrl_usdhc2_pins: usdhc2-gpiogrp {
> > > > +		fsl,pins = <
> > > > +			MX8MP_IOMUXC_SD2_CD_B__GPIO2_IO12	
> > > > 0x1c4
> > > > +			MX8MP_IOMUXC_SD2_RESET_B__GPIO2_IO19	
> > > > 0x4
> > > > 1
> > > > +		>;
> > > > +	};
> > > > +
> > > > +	pinctrl_usdhc2: usdhc2grp {
> > > > +		fsl,pins = <
> > > > +			MX8MP_IOMUXC_SD2_CLK__USDHC2_CLK	
> > > > 0x190
> > > > +			MX8MP_IOMUXC_SD2_CMD__USDHC2_CMD	
> > > > 0x1d0
> > > > +			MX8MP_IOMUXC_SD2_DATA0__USDHC2_DATA0	
> > > > 0x1
> > > > d0
> > > > +			MX8MP_IOMUXC_SD2_DATA1__USDHC2_DATA1	
> > > > 0x1
> > > > d0
> > > > +			MX8MP_IOMUXC_SD2_DATA2__USDHC2_DATA2	
> > > > 0x1
> > > > d0
> > > > +			MX8MP_IOMUXC_SD2_DATA3__USDHC2_DATA3	
> > > > 0x1
> > > > d0
> > > > +			MX8MP_IOMUXC_GPIO1_IO04__USDHC2_VSELECT
> > > > 	0xc
> > > > 1
> > > > +		>;
> > > > +	};
> > > > +
> > > > +	pinctrl_usdhc2_100mhz: usdhc2-100mhzgrp {
> > > > +		fsl,pins = <
> > > > +			MX8MP_IOMUXC_SD2_CLK__USDHC2_CLK	
> > > > 0x194
> > > > +			MX8MP_IOMUXC_SD2_CMD__USDHC2_CMD	
> > > > 0x1d4
> > > > +			MX8MP_IOMUXC_SD2_DATA0__USDHC2_DATA0	
> > > > 0x1
> > > > d4
> > > > +			MX8MP_IOMUXC_SD2_DATA1__USDHC2_DATA1	
> > > > 0x1
> > > > d4
> > > > +			MX8MP_IOMUXC_SD2_DATA2__USDHC2_DATA2	
> > > > 0x1
> > > > d4
> > > > +			MX8MP_IOMUXC_SD2_DATA3__USDHC2_DATA3	
> > > > 0x1
> > > > d4
> > > > +			MX8MP_IOMUXC_GPIO1_IO04__USDHC2_VSELECT
> > > > 	0xc
> > > > 1
> > > > +		>;
> > > > +	};
> > > > +
> > > > +	pinctrl_usdhc2_200mhz: usdhc2-200mhzgrp {
> > > > +		fsl,pins = <
> > > > +			MX8MP_IOMUXC_SD2_CLK__USDHC2_CLK	
> > > > 0x196
> > > > +			MX8MP_IOMUXC_SD2_CMD__USDHC2_CMD	
> > > > 0x1d6
> > > > +			MX8MP_IOMUXC_SD2_DATA0__USDHC2_DATA0	
> > > > 0x1
> > > > d6
> > > > +			MX8MP_IOMUXC_SD2_DATA1__USDHC2_DATA1	
> > > > 0x1
> > > > d6
> > > > +			MX8MP_IOMUXC_SD2_DATA2__USDHC2_DATA2	
> > > > 0x1
> > > > d6
> > > > +			MX8MP_IOMUXC_SD2_DATA3__USDHC2_DATA3	
> > > > 0x1
> > > > d6
> > > > +			MX8MP_IOMUXC_GPIO1_IO04__USDHC2_VSELECT
> > > > 	0xc
> > > > 1
> > > > +		>;
> > > > +	};
> > > > +};
> > > > diff --git a/arch/arm64/boot/dts/freescale/imx8mp-phycore-
> > > > som.dtsi
> > > > b/arch/arm64/boot/dts/freescale/imx8mp-phycore-som.dtsi
> > > > new file mode 100644
> > > > index 000000000000..e1fdfebd8142
> > > > --- /dev/null
> > > > +++ b/arch/arm64/boot/dts/freescale/imx8mp-phycore-som.dtsi
> > > > @@ -0,0 +1,319 @@
> > > > +// SPDX-License-Identifier: GPL-2.0
> > > > +/*
> > > > + * Copyright (C) 2020 PHYTEC Messtechnik GmbH
> > > > + * Author: Teresa Remmet <t.remmet@phytec.de>
> > > > + */
> > > > +
> > > > +#include <dt-bindings/net/ti-dp83867.h>
> > > > +#include "imx8mp.dtsi"
> > > > +
> > > > +/ {
> > > > +	model = "PHYTEC phyCORE-i.MX8MP";
> > > > +	compatible = "phytec,imx8mp-phycore-som", "fsl,imx8mp";
> > > > +
> > > > +	aliases {
> > > > +		rtc0 = &rv3028;
> > > > +		rtc1 = &snvs_rtc;
> > > > +	};
> > > > +
> > > > +	memory@40000000 {
> > > > +		device_type = "memory";
> > > > +		reg = <0x0 0x40000000 0 0x80000000>;
> > > > +	};
> > > > +
> > > > +	rtcclkout: rv3028-clkout {
> > > 
> > > Is it really a separate oscillator giving 32 kHz? Or maybe this
> > > is
> > > actually part of PMIC?
> > 
> > It is a clock out of the used i2c rtc. Which I actually trying to
> > disable. As it is not connected. But it is enabled as default.
> 
> This does not make sense at all:
> 1. This is a node without any reference to hardware,
> 2. It is being disabled in DTS so it will not have any effect in
> kernel
>    therefore will not have any impact on real hardware,

I measured it. I could see that the clock was being disabled. But yes
it does not feel like correct solution and needs more investigation.
I was not able to find the correct property modification.
Will remove this for now and find a proper solution afterwards. It does
not have impact on the functionality if it is enabled or not.
So I will remove the clock part in v2.

Regards,
Teresa

> 3. The RV3028 RTC takes it as phandle... but RV3028 RTC is a clock
>    provider, not a consumer.
> 
> If you want to disable RV3028 RTC clock, you need to modify it's
> properties, not add fake nodes.
> 
> Best regards,
> Krzysztof

Re: [PATCH 4/4] arm64: dts: freescale: Add support for phyBOARD-Pollux-i.MX8MP

[Teresa Remmet]

Hello Krzysztof,

Am Montag, den 07.12.2020, 14:46 +0100 schrieb Krzysztof Kozlowski:
> On Mon, Dec 07, 2020 at 02:35:33PM +0100, Teresa Remmet wrote:
> > Hello Krzysztof,
> > 
> > Am Montag, den 07.12.2020, 13:09 +0100 schrieb Krzysztof Kozlowski:
> > > On Fri, Dec 04, 2020 at 09:33:02PM +0100, Teresa Remmet wrote:
> > > > Add initial support for phyBOARD-Pollux-i.MX8MP.
> > > > Supported basic features:
> > > > 	* eMMC
> > > > 	* i2c EEPROM
> > > > 	* i2c RTC
> > > > 	* i2c LED
> > > > 	* PMIC
> > > > 	* debug UART
> > > > 	* SD card
> > > > 	* 1Gbit Ethernet (fec)
> > > > 	* watchdog
> > > > 
> > > > Signed-off-by: Teresa Remmet <t.remmet@phytec.de>
> > > > ---
> > > >  arch/arm64/boot/dts/freescale/Makefile             |   1 +
> > > >  .../dts/freescale/imx8mp-phyboard-pollux-rdk.dts   |  16 ++
> > > >  .../boot/dts/freescale/imx8mp-phyboard-pollux.dtsi | 152
> > > > ++++++++++
> > > >  .../boot/dts/freescale/imx8mp-phycore-som.dtsi     | 319
> > > > +++++++++++++++++++++
> > > >  4 files changed, 488 insertions(+)
> > > >  create mode 100644 arch/arm64/boot/dts/freescale/imx8mp-
> > > > phyboard-
> > > > pollux-rdk.dts
> > > >  create mode 100644 arch/arm64/boot/dts/freescale/imx8mp-
> > > > phyboard-
> > > > pollux.dtsi
> > > >  create mode 100644 arch/arm64/boot/dts/freescale/imx8mp-
> > > > phycore-
> > > > som.dtsi
> > > > 
> > > > diff --git a/arch/arm64/boot/dts/freescale/Makefile
> > > > b/arch/arm64/boot/dts/freescale/Makefile
> > > > index acfb8af45912..a43b496678be 100644
> > > > --- a/arch/arm64/boot/dts/freescale/Makefile
> > > > +++ b/arch/arm64/boot/dts/freescale/Makefile
> > > > @@ -37,6 +37,7 @@ dtb-$(CONFIG_ARCH_MXC) += imx8mn-evk.dtb
> > > >  dtb-$(CONFIG_ARCH_MXC) += imx8mn-ddr4-evk.dtb
> > > >  dtb-$(CONFIG_ARCH_MXC) += imx8mn-var-som-symphony.dtb
> > > >  dtb-$(CONFIG_ARCH_MXC) += imx8mp-evk.dtb
> > > > +dtb-$(CONFIG_ARCH_MXC) += imx8mp-phyboard-pollux-rdk.dtb
> > > >  dtb-$(CONFIG_ARCH_MXC) += imx8mq-evk.dtb
> > > >  dtb-$(CONFIG_ARCH_MXC) += imx8mq-hummingboard-pulse.dtb
> > > >  dtb-$(CONFIG_ARCH_MXC) += imx8mq-librem5-devkit.dtb
> > > > diff --git a/arch/arm64/boot/dts/freescale/imx8mp-phyboard-
> > > > pollux-
> > > > rdk.dts b/arch/arm64/boot/dts/freescale/imx8mp-phyboard-pollux-
> > > > rdk.dts
> > > > new file mode 100644
> > > > index 000000000000..dd64be32c99d
> > > > --- /dev/null
> > > > +++ b/arch/arm64/boot/dts/freescale/imx8mp-phyboard-pollux-
> > > > rdk.dts
> > > > @@ -0,0 +1,16 @@
> > > > +// SPDX-License-Identifier: GPL-2.0
> > > > +/*
> > > > + * Copyright (C) 2020 PHYTEC Messtechnik GmbH
> > > > + * Author: Teresa Remmet <t.remmet@phytec.de>
> > > > + */
> > > > +
> > > > +/dts-v1/;
> > > > +
> > > > +#include "imx8mp-phycore-som.dtsi"
> > > > +#include "imx8mp-phyboard-pollux.dtsi"
> > > > +
> > > > +/ {
> > > > +	model = "PHYTEC phyBOARD-Pollux i.MX8MP";
> > > > +	compatible = "phytec,imx8mp-phyboard-pollux-rdk",
> > > > +		     "phytec,imx8mp-phycore-som", "fsl,imx8mp";
> > > 
> > > This is the purpose of this file? Why having a DTS to include
> > > DTSI
> > > only?
> > > Usually there is just DTSI for SOM and DTS fot the board.
> > 
> > we have different options for the SoMs. Like SPI-NOR flash mounted
> > or
> > not. We usually add this to the SoM include, but disable it. We
> > enable
> > this in the dts if mounted. This makes it easy to generate
> > different
> > device trees for different SoM options. So far upstream is not
> > every
> > feature supported. So we don't do anything in the dts yet. But I
> > want
> > to setup the layout already.
> > 
> > I hope this makes it clear.
> 
> You make the upstream DTSes more complicated to make it easier for
> downstream. No, this does not work this way. You can either upstream
> other DTSes so such split will make sense, or this contribution
> should
> make sense in the upstreamed state.
> 
> In the second case, by "matching upstreamed state" I mean that you
> organize your DTSes in a way they make sense for upstream, for
> example
> one DTSI for the SOM and one DTS for the board using it.

Ok, then i will change it now like you suggested and rework it later
after more features are available.

> 
> > 
> > > 
> > > > +};
> > > > diff --git a/arch/arm64/boot/dts/freescale/imx8mp-phyboard-
> > > > pollux.dtsi b/arch/arm64/boot/dts/freescale/imx8mp-phyboard-
> > > > pollux.dtsi
> > > > new file mode 100644
> > > > index 000000000000..dbeaa27eb043
> > > > --- /dev/null
> > > > +++ b/arch/arm64/boot/dts/freescale/imx8mp-phyboard-pollux.dtsi
> > > > @@ -0,0 +1,152 @@
> > > > +// SPDX-License-Identifier: GPL-2.0
> > > > +/*
> > > > + * Copyright (C) 2020 PHYTEC Messtechnik GmbH
> > > > + * Author: Teresa Remmet <t.remmet@phytec.de>
> > > > + */
> > > > +
> > > > +#include <dt-bindings/leds/leds-pca9532.h>
> > > > +#include <dt-bindings/pwm/pwm.h>
> > > > +
> > > > +/ {
> > > > +	chosen {
> > > > +		stdout-path = &uart2;
> > > > +	};
> > > > +
> > > > +	reg_usdhc2_vmmc: regulator-usdhc2 {
> > > > +		compatible = "regulator-fixed";
> > > > +		regulator-name = "VSD_3V3";
> > > > +		regulator-min-microvolt = <3300000>;
> > > > +		regulator-max-microvolt = <3300000>;
> > > > +		gpio = <&gpio2 19 GPIO_ACTIVE_HIGH>;
> > > > +		enable-active-high;
> > > > +		startup-delay-us = <100>;
> > > > +		off-on-delay-us = <12000>;
> > > > +	};
> > > > +};
> > > > +
> > > > +&i2c2 {
> > > > +	clock-frequency = <400000>;
> > > > +	pinctrl-names = "default";
> > > > +	pinctrl-0 = <&pinctrl_i2c2>;
> > > > +	pinctrl-1 = <&pinctrl_i2c2_gpio>;
> > > > +	sda-gpios = <&gpio5 17 (GPIO_ACTIVE_HIGH |
> > > > GPIO_OPEN_DRAIN)>;
> > > > +	scl-gpios = <&gpio5 16 (GPIO_ACTIVE_HIGH |
> > > > GPIO_OPEN_DRAIN)>;
> > > > +	status = "okay";
> > > > +
> > > > +	eeprom@51 {
> > > > +		compatible = "atmel,24c02";
> > > > +		reg = <0x51>;
> > > > +		pagesize = <16>;
> > > > +		status = "okay";
> > > > +	};
> > > > +
> > > > +	leddimmer@62 {
> > > 
> > > I think name "leds" is more appropriate.
> > 
> > okay I will change it.
> > 
> > > 
> > > > +		compatible = "nxp,pca9533";
> > > > +		reg = <0x62>;
> > > > +		status = "okay";
> > > > +
> > > > +		led1 {
> > > > +			label = "red:user1";
> > > > +			type = <PCA9532_TYPE_LED>;
> > > > +		};
> > > > +
> > > > +		led2 {
> > > > +			label = "green:user2";
> > > > +			type = <PCA9532_TYPE_LED>;
> > > > +		};
> > > > +
> > > > +		led3 {
> > > > +			label = "blue:user3";
> > > > +			type = <PCA9532_TYPE_LED>;
> > > > +		};
> > > > +	};
> > > > +};
> > > > +
> > > > +&snvs_pwrkey {
> > > > +	status = "okay";
> > > > +};
> > > > +
> > > > +/* debug console */
> > > > +&uart2 {
> > > > +	pinctrl-names = "default";
> > > > +	pinctrl-0 = <&pinctrl_uart2>;
> > > > +	status = "okay";
> > > > +};
> > > > +
> > > > +/* SD-Card */
> > > > +&usdhc2 {
> > > > +	pinctrl-names = "default", "state_100mhz",
> > > > "state_200mhz";
> > > > +	pinctrl-0 = <&pinctrl_usdhc2>, <&pinctrl_usdhc2_pins>;
> > > > +	pinctrl-1 = <&pinctrl_usdhc2_100mhz>,
> > > > <&pinctrl_usdhc2_pins>;
> > > > +	pinctrl-2 = <&pinctrl_usdhc2_200mhz>,
> > > > <&pinctrl_usdhc2_pins>;
> > > > +	cd-gpios = <&gpio2 12 GPIO_ACTIVE_LOW>;
> > > > +	vmmc-supply = <&reg_usdhc2_vmmc>;
> > > > +	bus-width = <4>;
> > > > +	status = "okay";
> > > > +};
> > > > +
> > > > +&iomuxc {
> > > > +	pinctrl_i2c2: i2c2grp {
> > > > +		fsl,pins = <
> > > > +			MX8MP_IOMUXC_I2C2_SCL__I2C2_SCL		
> > > > 0x4
> > > > 00001c3
> > > > +			MX8MP_IOMUXC_I2C2_SDA__I2C2_SDA		
> > > > 0x4
> > > > 00001c3
> > > > +		>;
> > > > +	};
> > > > +
> > > > +	pinctrl_i2c2_gpio: i2c2gpiogrp {
> > > > +		fsl,pins = <
> > > > +			MX8MP_IOMUXC_I2C2_SCL__GPIO5_IO16	
> > > > 0x1e3
> > > > +			MX8MP_IOMUXC_I2C2_SDA__GPIO5_IO17	
> > > > 0x1e3
> > > > +		>;
> > > > +	};
> > > > +
> > > > +	pinctrl_uart2: uart2grp {
> > > > +		fsl,pins = <
> > > > +			MX8MP_IOMUXC_UART2_RXD__UART2_DCE_RX	
> > > > 0x4
> > > > 9
> > > > +			MX8MP_IOMUXC_UART2_TXD__UART2_DCE_TX	
> > > > 0x4
> > > > 9
> > > > +		>;
> > > > +	};
> > > > +
> > > > +	pinctrl_usdhc2_pins: usdhc2-gpiogrp {
> > > > +		fsl,pins = <
> > > > +			MX8MP_IOMUXC_SD2_CD_B__GPIO2_IO12	
> > > > 0x1c4
> > > > +			MX8MP_IOMUXC_SD2_RESET_B__GPIO2_IO19	
> > > > 0x4
> > > > 1
> > > > +		>;
> > > > +	};
> > > > +
> > > > +	pinctrl_usdhc2: usdhc2grp {
> > > > +		fsl,pins = <
> > > > +			MX8MP_IOMUXC_SD2_CLK__USDHC2_CLK	
> > > > 0x190
> > > > +			MX8MP_IOMUXC_SD2_CMD__USDHC2_CMD	
> > > > 0x1d0
> > > > +			MX8MP_IOMUXC_SD2_DATA0__USDHC2_DATA0	
> > > > 0x1
> > > > d0
> > > > +			MX8MP_IOMUXC_SD2_DATA1__USDHC2_DATA1	
> > > > 0x1
> > > > d0
> > > > +			MX8MP_IOMUXC_SD2_DATA2__USDHC2_DATA2	
> > > > 0x1
> > > > d0
> > > > +			MX8MP_IOMUXC_SD2_DATA3__USDHC2_DATA3	
> > > > 0x1
> > > > d0
> > > > +			MX8MP_IOMUXC_GPIO1_IO04__USDHC2_VSELECT
> > > > 	0xc
> > > > 1
> > > > +		>;
> > > > +	};
> > > > +
> > > > +	pinctrl_usdhc2_100mhz: usdhc2-100mhzgrp {
> > > > +		fsl,pins = <
> > > > +			MX8MP_IOMUXC_SD2_CLK__USDHC2_CLK	
> > > > 0x194
> > > > +			MX8MP_IOMUXC_SD2_CMD__USDHC2_CMD	
> > > > 0x1d4
> > > > +			MX8MP_IOMUXC_SD2_DATA0__USDHC2_DATA0	
> > > > 0x1
> > > > d4
> > > > +			MX8MP_IOMUXC_SD2_DATA1__USDHC2_DATA1	
> > > > 0x1
> > > > d4
> > > > +			MX8MP_IOMUXC_SD2_DATA2__USDHC2_DATA2	
> > > > 0x1
> > > > d4
> > > > +			MX8MP_IOMUXC_SD2_DATA3__USDHC2_DATA3	
> > > > 0x1
> > > > d4
> > > > +			MX8MP_IOMUXC_GPIO1_IO04__USDHC2_VSELECT
> > > > 	0xc
> > > > 1
> > > > +		>;
> > > > +	};
> > > > +
> > > > +	pinctrl_usdhc2_200mhz: usdhc2-200mhzgrp {
> > > > +		fsl,pins = <
> > > > +			MX8MP_IOMUXC_SD2_CLK__USDHC2_CLK	
> > > > 0x196
> > > > +			MX8MP_IOMUXC_SD2_CMD__USDHC2_CMD	
> > > > 0x1d6
> > > > +			MX8MP_IOMUXC_SD2_DATA0__USDHC2_DATA0	
> > > > 0x1
> > > > d6
> > > > +			MX8MP_IOMUXC_SD2_DATA1__USDHC2_DATA1	
> > > > 0x1
> > > > d6
> > > > +			MX8MP_IOMUXC_SD2_DATA2__USDHC2_DATA2	
> > > > 0x1
> > > > d6
> > > > +			MX8MP_IOMUXC_SD2_DATA3__USDHC2_DATA3	
> > > > 0x1
> > > > d6
> > > > +			MX8MP_IOMUXC_GPIO1_IO04__USDHC2_VSELECT
> > > > 	0xc
> > > > 1
> > > > +		>;
> > > > +	};
> > > > +};
> > > > diff --git a/arch/arm64/boot/dts/freescale/imx8mp-phycore-
> > > > som.dtsi
> > > > b/arch/arm64/boot/dts/freescale/imx8mp-phycore-som.dtsi
> > > > new file mode 100644
> > > > index 000000000000..e1fdfebd8142
> > > > --- /dev/null
> > > > +++ b/arch/arm64/boot/dts/freescale/imx8mp-phycore-som.dtsi
> > > > @@ -0,0 +1,319 @@
> > > > +// SPDX-License-Identifier: GPL-2.0
> > > > +/*
> > > > + * Copyright (C) 2020 PHYTEC Messtechnik GmbH
> > > > + * Author: Teresa Remmet <t.remmet@phytec.de>
> > > > + */
> > > > +
> > > > +#include <dt-bindings/net/ti-dp83867.h>
> > > > +#include "imx8mp.dtsi"
> > > > +
> > > > +/ {
> > > > +	model = "PHYTEC phyCORE-i.MX8MP";
> > > > +	compatible = "phytec,imx8mp-phycore-som", "fsl,imx8mp";
> > > > +
> > > > +	aliases {
> > > > +		rtc0 = &rv3028;
> > > > +		rtc1 = &snvs_rtc;
> > > > +	};
> > > > +
> > > > +	memory@40000000 {
> > > > +		device_type = "memory";
> > > > +		reg = <0x0 0x40000000 0 0x80000000>;
> > > > +	};
> > > > +
> > > > +	rtcclkout: rv3028-clkout {
> > > 
> > > Is it really a separate oscillator giving 32 kHz? Or maybe this
> > > is
> > > actually part of PMIC?
> > 
> > It is a clock out of the used i2c rtc. Which I actually trying to
> > disable. As it is not connected. But it is enabled as default.
> 
> This does not make sense at all:
> 1. This is a node without any reference to hardware,
> 2. It is being disabled in DTS so it will not have any effect in
> kernel
>    therefore will not have any impact on real hardware,

I measured it. I could see that the clock was being disabled. But yes
it does not feel like correct solution and needs more investigation.
I was not able to find the correct property modification.
Will remove this for now and find a proper solution afterwards. It does
not have impact on the functionality if it is enabled or not.
So I will remove the clock part in v2.

Regards,
Teresa

> 3. The RV3028 RTC takes it as phandle... but RV3028 RTC is a clock
>    provider, not a consumer.
> 
> If you want to disable RV3028 RTC clock, you need to modify it's
> properties, not add fake nodes.
> 
> Best regards,
> Krzysztof


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 4/4] arm64: dts: freescale: Add support for phyBOARD-Pollux-i.MX8MP

[Krzysztof Kozlowski]

On Tue, Dec 08, 2020 at 12:53:22PM +0100, Teresa Remmet wrote:
> Hello Krzysztof,
> 
> Am Montag, den 07.12.2020, 14:46 +0100 schrieb Krzysztof Kozlowski:
> > On Mon, Dec 07, 2020 at 02:35:33PM +0100, Teresa Remmet wrote:
> > > Hello Krzysztof,
> > > 
> > > Am Montag, den 07.12.2020, 13:09 +0100 schrieb Krzysztof Kozlowski:
> > > > On Fri, Dec 04, 2020 at 09:33:02PM +0100, Teresa Remmet wrote:
> > > > > Add initial support for phyBOARD-Pollux-i.MX8MP.
> > > > > Supported basic features:
> > > > > 	* eMMC
> > > > > 	* i2c EEPROM
> > > > > 	* i2c RTC
> > > > > 	* i2c LED
> > > > > 	* PMIC
> > > > > 	* debug UART
> > > > > 	* SD card
> > > > > 	* 1Gbit Ethernet (fec)
> > > > > 	* watchdog
> > > > > 
> > > > > Signed-off-by: Teresa Remmet <t.remmet@phytec.de>
> > > > > ---
> > > > >  arch/arm64/boot/dts/freescale/Makefile             |   1 +
> > > > >  .../dts/freescale/imx8mp-phyboard-pollux-rdk.dts   |  16 ++
> > > > >  .../boot/dts/freescale/imx8mp-phyboard-pollux.dtsi | 152
> > > > > ++++++++++
> > > > >  .../boot/dts/freescale/imx8mp-phycore-som.dtsi     | 319
> > > > > +++++++++++++++++++++
> > > > >  4 files changed, 488 insertions(+)
> > > > >  create mode 100644 arch/arm64/boot/dts/freescale/imx8mp-
> > > > > phyboard-
> > > > > pollux-rdk.dts
> > > > >  create mode 100644 arch/arm64/boot/dts/freescale/imx8mp-
> > > > > phyboard-
> > > > > pollux.dtsi
> > > > >  create mode 100644 arch/arm64/boot/dts/freescale/imx8mp-
> > > > > phycore-
> > > > > som.dtsi
> > > > > 
> > > > > diff --git a/arch/arm64/boot/dts/freescale/Makefile
> > > > > b/arch/arm64/boot/dts/freescale/Makefile
> > > > > index acfb8af45912..a43b496678be 100644
> > > > > --- a/arch/arm64/boot/dts/freescale/Makefile
> > > > > +++ b/arch/arm64/boot/dts/freescale/Makefile
> > > > > @@ -37,6 +37,7 @@ dtb-$(CONFIG_ARCH_MXC) += imx8mn-evk.dtb
> > > > >  dtb-$(CONFIG_ARCH_MXC) += imx8mn-ddr4-evk.dtb
> > > > >  dtb-$(CONFIG_ARCH_MXC) += imx8mn-var-som-symphony.dtb
> > > > >  dtb-$(CONFIG_ARCH_MXC) += imx8mp-evk.dtb
> > > > > +dtb-$(CONFIG_ARCH_MXC) += imx8mp-phyboard-pollux-rdk.dtb
> > > > >  dtb-$(CONFIG_ARCH_MXC) += imx8mq-evk.dtb
> > > > >  dtb-$(CONFIG_ARCH_MXC) += imx8mq-hummingboard-pulse.dtb
> > > > >  dtb-$(CONFIG_ARCH_MXC) += imx8mq-librem5-devkit.dtb
> > > > > diff --git a/arch/arm64/boot/dts/freescale/imx8mp-phyboard-
> > > > > pollux-
> > > > > rdk.dts b/arch/arm64/boot/dts/freescale/imx8mp-phyboard-pollux-
> > > > > rdk.dts
> > > > > new file mode 100644
> > > > > index 000000000000..dd64be32c99d
> > > > > --- /dev/null
> > > > > +++ b/arch/arm64/boot/dts/freescale/imx8mp-phyboard-pollux-
> > > > > rdk.dts
> > > > > @@ -0,0 +1,16 @@
> > > > > +// SPDX-License-Identifier: GPL-2.0
> > > > > +/*
> > > > > + * Copyright (C) 2020 PHYTEC Messtechnik GmbH
> > > > > + * Author: Teresa Remmet <t.remmet@phytec.de>
> > > > > + */
> > > > > +
> > > > > +/dts-v1/;
> > > > > +
> > > > > +#include "imx8mp-phycore-som.dtsi"
> > > > > +#include "imx8mp-phyboard-pollux.dtsi"
> > > > > +
> > > > > +/ {
> > > > > +	model = "PHYTEC phyBOARD-Pollux i.MX8MP";
> > > > > +	compatible = "phytec,imx8mp-phyboard-pollux-rdk",
> > > > > +		     "phytec,imx8mp-phycore-som", "fsl,imx8mp";
> > > > 
> > > > This is the purpose of this file? Why having a DTS to include
> > > > DTSI
> > > > only?
> > > > Usually there is just DTSI for SOM and DTS fot the board.
> > > 
> > > we have different options for the SoMs. Like SPI-NOR flash mounted
> > > or
> > > not. We usually add this to the SoM include, but disable it. We
> > > enable
> > > this in the dts if mounted. This makes it easy to generate
> > > different
> > > device trees for different SoM options. So far upstream is not
> > > every
> > > feature supported. So we don't do anything in the dts yet. But I
> > > want
> > > to setup the layout already.
> > > 
> > > I hope this makes it clear.
> > 
> > You make the upstream DTSes more complicated to make it easier for
> > downstream. No, this does not work this way. You can either upstream
> > other DTSes so such split will make sense, or this contribution
> > should
> > make sense in the upstreamed state.
> > 
> > In the second case, by "matching upstreamed state" I mean that you
> > organize your DTSes in a way they make sense for upstream, for
> > example
> > one DTSI for the SOM and one DTS for the board using it.
> 
> Ok, then i will change it now like you suggested and rework it later
> after more features are available.

If you submit two DTSes using the phyboard DTSI, it will be enough to
justify that split.

[...]

> > > > > +	rtcclkout: rv3028-clkout {
> > > > 
> > > > Is it really a separate oscillator giving 32 kHz? Or maybe this
> > > > is
> > > > actually part of PMIC?
> > > 
> > > It is a clock out of the used i2c rtc. Which I actually trying to
> > > disable. As it is not connected. But it is enabled as default.
> > 
> > This does not make sense at all:
> > 1. This is a node without any reference to hardware,
> > 2. It is being disabled in DTS so it will not have any effect in
> > kernel
> >    therefore will not have any impact on real hardware,
> 
> I measured it. I could see that the clock was being disabled. But yes
> it does not feel like correct solution and needs more investigation.
> I was not able to find the correct property modification.
> Will remove this for now and find a proper solution afterwards. It does
> not have impact on the functionality if it is enabled or not.
> So I will remove the clock part in v2.

Mhmm... I assume you also measured it without this clock-dance in DTS
and the clock was on in such case?

It is pretty confusing... The RV3028 registers a clock provider and its
clock will be disabled by the core because it is not used. Adding a
clock consumer to RV3028 should not change here anything because RV3028
does not use this clock. Adding a fixed clock without reference to HW
also should not change here anything.

Anyway, your RV3028 node with a clock phandle would not pass dtschema
check so it's a hint you are doing something not correct for Linux
kernel.

Best regards,
Krzysztof

Re: [PATCH 4/4] arm64: dts: freescale: Add support for phyBOARD-Pollux-i.MX8MP

[Krzysztof Kozlowski]

On Tue, Dec 08, 2020 at 12:53:22PM +0100, Teresa Remmet wrote:
> Hello Krzysztof,
> 
> Am Montag, den 07.12.2020, 14:46 +0100 schrieb Krzysztof Kozlowski:
> > On Mon, Dec 07, 2020 at 02:35:33PM +0100, Teresa Remmet wrote:
> > > Hello Krzysztof,
> > > 
> > > Am Montag, den 07.12.2020, 13:09 +0100 schrieb Krzysztof Kozlowski:
> > > > On Fri, Dec 04, 2020 at 09:33:02PM +0100, Teresa Remmet wrote:
> > > > > Add initial support for phyBOARD-Pollux-i.MX8MP.
> > > > > Supported basic features:
> > > > > 	* eMMC
> > > > > 	* i2c EEPROM
> > > > > 	* i2c RTC
> > > > > 	* i2c LED
> > > > > 	* PMIC
> > > > > 	* debug UART
> > > > > 	* SD card
> > > > > 	* 1Gbit Ethernet (fec)
> > > > > 	* watchdog
> > > > > 
> > > > > Signed-off-by: Teresa Remmet <t.remmet@phytec.de>
> > > > > ---
> > > > >  arch/arm64/boot/dts/freescale/Makefile             |   1 +
> > > > >  .../dts/freescale/imx8mp-phyboard-pollux-rdk.dts   |  16 ++
> > > > >  .../boot/dts/freescale/imx8mp-phyboard-pollux.dtsi | 152
> > > > > ++++++++++
> > > > >  .../boot/dts/freescale/imx8mp-phycore-som.dtsi     | 319
> > > > > +++++++++++++++++++++
> > > > >  4 files changed, 488 insertions(+)
> > > > >  create mode 100644 arch/arm64/boot/dts/freescale/imx8mp-
> > > > > phyboard-
> > > > > pollux-rdk.dts
> > > > >  create mode 100644 arch/arm64/boot/dts/freescale/imx8mp-
> > > > > phyboard-
> > > > > pollux.dtsi
> > > > >  create mode 100644 arch/arm64/boot/dts/freescale/imx8mp-
> > > > > phycore-
> > > > > som.dtsi
> > > > > 
> > > > > diff --git a/arch/arm64/boot/dts/freescale/Makefile
> > > > > b/arch/arm64/boot/dts/freescale/Makefile
> > > > > index acfb8af45912..a43b496678be 100644
> > > > > --- a/arch/arm64/boot/dts/freescale/Makefile
> > > > > +++ b/arch/arm64/boot/dts/freescale/Makefile
> > > > > @@ -37,6 +37,7 @@ dtb-$(CONFIG_ARCH_MXC) += imx8mn-evk.dtb
> > > > >  dtb-$(CONFIG_ARCH_MXC) += imx8mn-ddr4-evk.dtb
> > > > >  dtb-$(CONFIG_ARCH_MXC) += imx8mn-var-som-symphony.dtb
> > > > >  dtb-$(CONFIG_ARCH_MXC) += imx8mp-evk.dtb
> > > > > +dtb-$(CONFIG_ARCH_MXC) += imx8mp-phyboard-pollux-rdk.dtb
> > > > >  dtb-$(CONFIG_ARCH_MXC) += imx8mq-evk.dtb
> > > > >  dtb-$(CONFIG_ARCH_MXC) += imx8mq-hummingboard-pulse.dtb
> > > > >  dtb-$(CONFIG_ARCH_MXC) += imx8mq-librem5-devkit.dtb
> > > > > diff --git a/arch/arm64/boot/dts/freescale/imx8mp-phyboard-
> > > > > pollux-
> > > > > rdk.dts b/arch/arm64/boot/dts/freescale/imx8mp-phyboard-pollux-
> > > > > rdk.dts
> > > > > new file mode 100644
> > > > > index 000000000000..dd64be32c99d
> > > > > --- /dev/null
> > > > > +++ b/arch/arm64/boot/dts/freescale/imx8mp-phyboard-pollux-
> > > > > rdk.dts
> > > > > @@ -0,0 +1,16 @@
> > > > > +// SPDX-License-Identifier: GPL-2.0
> > > > > +/*
> > > > > + * Copyright (C) 2020 PHYTEC Messtechnik GmbH
> > > > > + * Author: Teresa Remmet <t.remmet@phytec.de>
> > > > > + */
> > > > > +
> > > > > +/dts-v1/;
> > > > > +
> > > > > +#include "imx8mp-phycore-som.dtsi"
> > > > > +#include "imx8mp-phyboard-pollux.dtsi"
> > > > > +
> > > > > +/ {
> > > > > +	model = "PHYTEC phyBOARD-Pollux i.MX8MP";
> > > > > +	compatible = "phytec,imx8mp-phyboard-pollux-rdk",
> > > > > +		     "phytec,imx8mp-phycore-som", "fsl,imx8mp";
> > > > 
> > > > This is the purpose of this file? Why having a DTS to include
> > > > DTSI
> > > > only?
> > > > Usually there is just DTSI for SOM and DTS fot the board.
> > > 
> > > we have different options for the SoMs. Like SPI-NOR flash mounted
> > > or
> > > not. We usually add this to the SoM include, but disable it. We
> > > enable
> > > this in the dts if mounted. This makes it easy to generate
> > > different
> > > device trees for different SoM options. So far upstream is not
> > > every
> > > feature supported. So we don't do anything in the dts yet. But I
> > > want
> > > to setup the layout already.
> > > 
> > > I hope this makes it clear.
> > 
> > You make the upstream DTSes more complicated to make it easier for
> > downstream. No, this does not work this way. You can either upstream
> > other DTSes so such split will make sense, or this contribution
> > should
> > make sense in the upstreamed state.
> > 
> > In the second case, by "matching upstreamed state" I mean that you
> > organize your DTSes in a way they make sense for upstream, for
> > example
> > one DTSI for the SOM and one DTS for the board using it.
> 
> Ok, then i will change it now like you suggested and rework it later
> after more features are available.

If you submit two DTSes using the phyboard DTSI, it will be enough to
justify that split.

[...]

> > > > > +	rtcclkout: rv3028-clkout {
> > > > 
> > > > Is it really a separate oscillator giving 32 kHz? Or maybe this
> > > > is
> > > > actually part of PMIC?
> > > 
> > > It is a clock out of the used i2c rtc. Which I actually trying to
> > > disable. As it is not connected. But it is enabled as default.
> > 
> > This does not make sense at all:
> > 1. This is a node without any reference to hardware,
> > 2. It is being disabled in DTS so it will not have any effect in
> > kernel
> >    therefore will not have any impact on real hardware,
> 
> I measured it. I could see that the clock was being disabled. But yes
> it does not feel like correct solution and needs more investigation.
> I was not able to find the correct property modification.
> Will remove this for now and find a proper solution afterwards. It does
> not have impact on the functionality if it is enabled or not.
> So I will remove the clock part in v2.

Mhmm... I assume you also measured it without this clock-dance in DTS
and the clock was on in such case?

It is pretty confusing... The RV3028 registers a clock provider and its
clock will be disabled by the core because it is not used. Adding a
clock consumer to RV3028 should not change here anything because RV3028
does not use this clock. Adding a fixed clock without reference to HW
also should not change here anything.

Anyway, your RV3028 node with a clock phandle would not pass dtschema
check so it's a hint you are doing something not correct for Linux
kernel.

Best regards,
Krzysztof


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 4/4] arm64: dts: freescale: Add support for phyBOARD-Pollux-i.MX8MP

[Teresa Remmet]

Hello Krzysztof,

Am Dienstag, den 08.12.2020, 13:00 +0100 schrieb Krzysztof Kozlowski:
> On Tue, Dec 08, 2020 at 12:53:22PM +0100, Teresa Remmet wrote:
> > Hello Krzysztof,
> > 
> > Am Montag, den 07.12.2020, 14:46 +0100 schrieb Krzysztof Kozlowski:
> > > On Mon, Dec 07, 2020 at 02:35:33PM +0100, Teresa Remmet wrote:
> > > > Hello Krzysztof,
> > > > 
> > > > Am Montag, den 07.12.2020, 13:09 +0100 schrieb Krzysztof
> > > > Kozlowski:
> > > > > On Fri, Dec 04, 2020 at 09:33:02PM +0100, Teresa Remmet
> > > > > wrote:
> > > > > > Add initial support for phyBOARD-Pollux-i.MX8MP.
> > > > > > Supported basic features:
> > > > > > 	* eMMC
> > > > > > 	* i2c EEPROM
> > > > > > 	* i2c RTC
> > > > > > 	* i2c LED
> > > > > > 	* PMIC
> > > > > > 	* debug UART
> > > > > > 	* SD card
> > > > > > 	* 1Gbit Ethernet (fec)
> > > > > > 	* watchdog
> > > > > > 
> > > > > > Signed-off-by: Teresa Remmet <t.remmet@phytec.de>
> > > > > > ---
> > > > > >  arch/arm64/boot/dts/freescale/Makefile             |   1 +
> > > > > >  .../dts/freescale/imx8mp-phyboard-pollux-rdk.dts   |  16
> > > > > > ++
> > > > > >  .../boot/dts/freescale/imx8mp-phyboard-pollux.dtsi | 152
> > > > > > ++++++++++
> > > > > >  .../boot/dts/freescale/imx8mp-phycore-som.dtsi     | 319
> > > > > > +++++++++++++++++++++
> > > > > >  4 files changed, 488 insertions(+)
> > > > > >  create mode 100644 arch/arm64/boot/dts/freescale/imx8mp-
> > > > > > phyboard-
> > > > > > pollux-rdk.dts
> > > > > >  create mode 100644 arch/arm64/boot/dts/freescale/imx8mp-
> > > > > > phyboard-
> > > > > > pollux.dtsi
> > > > > >  create mode 100644 arch/arm64/boot/dts/freescale/imx8mp-
> > > > > > phycore-
> > > > > > som.dtsi
> > > > > > 
> > > > > > diff --git a/arch/arm64/boot/dts/freescale/Makefile
> > > > > > b/arch/arm64/boot/dts/freescale/Makefile
> > > > > > index acfb8af45912..a43b496678be 100644
> > > > > > --- a/arch/arm64/boot/dts/freescale/Makefile
> > > > > > +++ b/arch/arm64/boot/dts/freescale/Makefile
> > > > > > @@ -37,6 +37,7 @@ dtb-$(CONFIG_ARCH_MXC) += imx8mn-evk.dtb
> > > > > >  dtb-$(CONFIG_ARCH_MXC) += imx8mn-ddr4-evk.dtb
> > > > > >  dtb-$(CONFIG_ARCH_MXC) += imx8mn-var-som-symphony.dtb
> > > > > >  dtb-$(CONFIG_ARCH_MXC) += imx8mp-evk.dtb
> > > > > > +dtb-$(CONFIG_ARCH_MXC) += imx8mp-phyboard-pollux-rdk.dtb
> > > > > >  dtb-$(CONFIG_ARCH_MXC) += imx8mq-evk.dtb
> > > > > >  dtb-$(CONFIG_ARCH_MXC) += imx8mq-hummingboard-pulse.dtb
> > > > > >  dtb-$(CONFIG_ARCH_MXC) += imx8mq-librem5-devkit.dtb
> > > > > > diff --git a/arch/arm64/boot/dts/freescale/imx8mp-phyboard-
> > > > > > pollux-
> > > > > > rdk.dts b/arch/arm64/boot/dts/freescale/imx8mp-phyboard-
> > > > > > pollux-
> > > > > > rdk.dts
> > > > > > new file mode 100644
> > > > > > index 000000000000..dd64be32c99d
> > > > > > --- /dev/null
> > > > > > +++ b/arch/arm64/boot/dts/freescale/imx8mp-phyboard-pollux-
> > > > > > rdk.dts
> > > > > > @@ -0,0 +1,16 @@
> > > > > > +// SPDX-License-Identifier: GPL-2.0
> > > > > > +/*
> > > > > > + * Copyright (C) 2020 PHYTEC Messtechnik GmbH
> > > > > > + * Author: Teresa Remmet <t.remmet@phytec.de>
> > > > > > + */
> > > > > > +
> > > > > > +/dts-v1/;
> > > > > > +
> > > > > > +#include "imx8mp-phycore-som.dtsi"
> > > > > > +#include "imx8mp-phyboard-pollux.dtsi"
> > > > > > +
> > > > > > +/ {
> > > > > > +	model = "PHYTEC phyBOARD-Pollux i.MX8MP";
> > > > > > +	compatible = "phytec,imx8mp-phyboard-pollux-rdk",
> > > > > > +		     "phytec,imx8mp-phycore-som", "fsl,imx8mp";
> > > > > 
> > > > > This is the purpose of this file? Why having a DTS to include
> > > > > DTSI
> > > > > only?
> > > > > Usually there is just DTSI for SOM and DTS fot the board.
> > > > 
> > > > we have different options for the SoMs. Like SPI-NOR flash
> > > > mounted
> > > > or
> > > > not. We usually add this to the SoM include, but disable it. We
> > > > enable
> > > > this in the dts if mounted. This makes it easy to generate
> > > > different
> > > > device trees for different SoM options. So far upstream is not
> > > > every
> > > > feature supported. So we don't do anything in the dts yet. But
> > > > I
> > > > want
> > > > to setup the layout already.
> > > > 
> > > > I hope this makes it clear.
> > > 
> > > You make the upstream DTSes more complicated to make it easier
> > > for
> > > downstream. No, this does not work this way. You can either
> > > upstream
> > > other DTSes so such split will make sense, or this contribution
> > > should
> > > make sense in the upstreamed state.
> > > 
> > > In the second case, by "matching upstreamed state" I mean that
> > > you
> > > organize your DTSes in a way they make sense for upstream, for
> > > example
> > > one DTSI for the SOM and one DTS for the board using it.
> > 
> > Ok, then i will change it now like you suggested and rework it
> > later
> > after more features are available.
> 
> If you submit two DTSes using the phyboard DTSI, it will be enough to
> justify that split.

Yes, but I don't have any features yet where the DTS files would differ
. So this would not make sense now.

> 
> [...]
> 
> > > > > > +	rtcclkout: rv3028-clkout {
> > > > > 
> > > > > Is it really a separate oscillator giving 32 kHz? Or maybe
> > > > > this
> > > > > is
> > > > > actually part of PMIC?
> > > > 
> > > > It is a clock out of the used i2c rtc. Which I actually trying
> > > > to
> > > > disable. As it is not connected. But it is enabled as default.
> > > 
> > > This does not make sense at all:
> > > 1. This is a node without any reference to hardware,
> > > 2. It is being disabled in DTS so it will not have any effect in
> > > kernel
> > >    therefore will not have any impact on real hardware,
> > 
> > I measured it. I could see that the clock was being disabled. But
> > yes
> > it does not feel like correct solution and needs more
> > investigation.
> > I was not able to find the correct property modification.
> > Will remove this for now and find a proper solution afterwards. It
> > does
> > not have impact on the functionality if it is enabled or not.
> > So I will remove the clock part in v2.
> 
> Mhmm... I assume you also measured it without this clock-dance in DTS
> and the clock was on in such case?

Yes, the clock was on. 

> 
> It is pretty confusing... The RV3028 registers a clock provider and
> its
> clock will be disabled by the core because it is not used. Adding a
> clock consumer to RV3028 should not change here anything because
> RV3028
> does not use this clock. Adding a fixed clock without reference to HW
> also should not change here anything.

True. I will need to investigate here more why it is not being disabled
as it is not used.

Teresa

> 
> Anyway, your RV3028 node with a clock phandle would not pass dtschema
> check so it's a hint you are doing something not correct for Linux
> kernel.
> 
> Best regards,
> Krzysztof
> 

Re: [PATCH 4/4] arm64: dts: freescale: Add support for phyBOARD-Pollux-i.MX8MP

[Teresa Remmet]

Hello Krzysztof,

Am Dienstag, den 08.12.2020, 13:00 +0100 schrieb Krzysztof Kozlowski:
> On Tue, Dec 08, 2020 at 12:53:22PM +0100, Teresa Remmet wrote:
> > Hello Krzysztof,
> > 
> > Am Montag, den 07.12.2020, 14:46 +0100 schrieb Krzysztof Kozlowski:
> > > On Mon, Dec 07, 2020 at 02:35:33PM +0100, Teresa Remmet wrote:
> > > > Hello Krzysztof,
> > > > 
> > > > Am Montag, den 07.12.2020, 13:09 +0100 schrieb Krzysztof
> > > > Kozlowski:
> > > > > On Fri, Dec 04, 2020 at 09:33:02PM +0100, Teresa Remmet
> > > > > wrote:
> > > > > > Add initial support for phyBOARD-Pollux-i.MX8MP.
> > > > > > Supported basic features:
> > > > > > 	* eMMC
> > > > > > 	* i2c EEPROM
> > > > > > 	* i2c RTC
> > > > > > 	* i2c LED
> > > > > > 	* PMIC
> > > > > > 	* debug UART
> > > > > > 	* SD card
> > > > > > 	* 1Gbit Ethernet (fec)
> > > > > > 	* watchdog
> > > > > > 
> > > > > > Signed-off-by: Teresa Remmet <t.remmet@phytec.de>
> > > > > > ---
> > > > > >  arch/arm64/boot/dts/freescale/Makefile             |   1 +
> > > > > >  .../dts/freescale/imx8mp-phyboard-pollux-rdk.dts   |  16
> > > > > > ++
> > > > > >  .../boot/dts/freescale/imx8mp-phyboard-pollux.dtsi | 152
> > > > > > ++++++++++
> > > > > >  .../boot/dts/freescale/imx8mp-phycore-som.dtsi     | 319
> > > > > > +++++++++++++++++++++
> > > > > >  4 files changed, 488 insertions(+)
> > > > > >  create mode 100644 arch/arm64/boot/dts/freescale/imx8mp-
> > > > > > phyboard-
> > > > > > pollux-rdk.dts
> > > > > >  create mode 100644 arch/arm64/boot/dts/freescale/imx8mp-
> > > > > > phyboard-
> > > > > > pollux.dtsi
> > > > > >  create mode 100644 arch/arm64/boot/dts/freescale/imx8mp-
> > > > > > phycore-
> > > > > > som.dtsi
> > > > > > 
> > > > > > diff --git a/arch/arm64/boot/dts/freescale/Makefile
> > > > > > b/arch/arm64/boot/dts/freescale/Makefile
> > > > > > index acfb8af45912..a43b496678be 100644
> > > > > > --- a/arch/arm64/boot/dts/freescale/Makefile
> > > > > > +++ b/arch/arm64/boot/dts/freescale/Makefile
> > > > > > @@ -37,6 +37,7 @@ dtb-$(CONFIG_ARCH_MXC) += imx8mn-evk.dtb
> > > > > >  dtb-$(CONFIG_ARCH_MXC) += imx8mn-ddr4-evk.dtb
> > > > > >  dtb-$(CONFIG_ARCH_MXC) += imx8mn-var-som-symphony.dtb
> > > > > >  dtb-$(CONFIG_ARCH_MXC) += imx8mp-evk.dtb
> > > > > > +dtb-$(CONFIG_ARCH_MXC) += imx8mp-phyboard-pollux-rdk.dtb
> > > > > >  dtb-$(CONFIG_ARCH_MXC) += imx8mq-evk.dtb
> > > > > >  dtb-$(CONFIG_ARCH_MXC) += imx8mq-hummingboard-pulse.dtb
> > > > > >  dtb-$(CONFIG_ARCH_MXC) += imx8mq-librem5-devkit.dtb
> > > > > > diff --git a/arch/arm64/boot/dts/freescale/imx8mp-phyboard-
> > > > > > pollux-
> > > > > > rdk.dts b/arch/arm64/boot/dts/freescale/imx8mp-phyboard-
> > > > > > pollux-
> > > > > > rdk.dts
> > > > > > new file mode 100644
> > > > > > index 000000000000..dd64be32c99d
> > > > > > --- /dev/null
> > > > > > +++ b/arch/arm64/boot/dts/freescale/imx8mp-phyboard-pollux-
> > > > > > rdk.dts
> > > > > > @@ -0,0 +1,16 @@
> > > > > > +// SPDX-License-Identifier: GPL-2.0
> > > > > > +/*
> > > > > > + * Copyright (C) 2020 PHYTEC Messtechnik GmbH
> > > > > > + * Author: Teresa Remmet <t.remmet@phytec.de>
> > > > > > + */
> > > > > > +
> > > > > > +/dts-v1/;
> > > > > > +
> > > > > > +#include "imx8mp-phycore-som.dtsi"
> > > > > > +#include "imx8mp-phyboard-pollux.dtsi"
> > > > > > +
> > > > > > +/ {
> > > > > > +	model = "PHYTEC phyBOARD-Pollux i.MX8MP";
> > > > > > +	compatible = "phytec,imx8mp-phyboard-pollux-rdk",
> > > > > > +		     "phytec,imx8mp-phycore-som", "fsl,imx8mp";
> > > > > 
> > > > > This is the purpose of this file? Why having a DTS to include
> > > > > DTSI
> > > > > only?
> > > > > Usually there is just DTSI for SOM and DTS fot the board.
> > > > 
> > > > we have different options for the SoMs. Like SPI-NOR flash
> > > > mounted
> > > > or
> > > > not. We usually add this to the SoM include, but disable it. We
> > > > enable
> > > > this in the dts if mounted. This makes it easy to generate
> > > > different
> > > > device trees for different SoM options. So far upstream is not
> > > > every
> > > > feature supported. So we don't do anything in the dts yet. But
> > > > I
> > > > want
> > > > to setup the layout already.
> > > > 
> > > > I hope this makes it clear.
> > > 
> > > You make the upstream DTSes more complicated to make it easier
> > > for
> > > downstream. No, this does not work this way. You can either
> > > upstream
> > > other DTSes so such split will make sense, or this contribution
> > > should
> > > make sense in the upstreamed state.
> > > 
> > > In the second case, by "matching upstreamed state" I mean that
> > > you
> > > organize your DTSes in a way they make sense for upstream, for
> > > example
> > > one DTSI for the SOM and one DTS for the board using it.
> > 
> > Ok, then i will change it now like you suggested and rework it
> > later
> > after more features are available.
> 
> If you submit two DTSes using the phyboard DTSI, it will be enough to
> justify that split.

Yes, but I don't have any features yet where the DTS files would differ
. So this would not make sense now.

> 
> [...]
> 
> > > > > > +	rtcclkout: rv3028-clkout {
> > > > > 
> > > > > Is it really a separate oscillator giving 32 kHz? Or maybe
> > > > > this
> > > > > is
> > > > > actually part of PMIC?
> > > > 
> > > > It is a clock out of the used i2c rtc. Which I actually trying
> > > > to
> > > > disable. As it is not connected. But it is enabled as default.
> > > 
> > > This does not make sense at all:
> > > 1. This is a node without any reference to hardware,
> > > 2. It is being disabled in DTS so it will not have any effect in
> > > kernel
> > >    therefore will not have any impact on real hardware,
> > 
> > I measured it. I could see that the clock was being disabled. But
> > yes
> > it does not feel like correct solution and needs more
> > investigation.
> > I was not able to find the correct property modification.
> > Will remove this for now and find a proper solution afterwards. It
> > does
> > not have impact on the functionality if it is enabled or not.
> > So I will remove the clock part in v2.
> 
> Mhmm... I assume you also measured it without this clock-dance in DTS
> and the clock was on in such case?

Yes, the clock was on. 

> 
> It is pretty confusing... The RV3028 registers a clock provider and
> its
> clock will be disabled by the core because it is not used. Adding a
> clock consumer to RV3028 should not change here anything because
> RV3028
> does not use this clock. Adding a fixed clock without reference to HW
> also should not change here anything.

True. I will need to investigate here more why it is not being disabled
as it is not used.

Teresa

> 
> Anyway, your RV3028 node with a clock phandle would not pass dtschema
> check so it's a hint you are doing something not correct for Linux
> kernel.
> 
> Best regards,
> Krzysztof
> 


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel