mbox series

[00/12] Add support for Orange Pi 3

Message ID 20190405234514.6183-1-megous@megous.com
Headers show
Series Add support for Orange Pi 3 | expand

Message

Ondřej Jirman April 5, 2019, 11:45 p.m. UTC
From: Ondrej Jirman <megous@megous.com>

This series implements support for Xunlong Orange Pi 3 board.

Unfortunately, this board needs some small driver patches, so I have
split the boards DT patch into chunks that require patches for drivers
in various subsystems:

- Basic DT for the board  (patch 1)
- HDMI support            (patches 2, 3, 4)
- Ethernet support        (patches 5, 6, 7)
- WiFi support            (patches 8, 9, 10, 11, 12)

This patch is also needed to not get segfault on boot: 
  https://lkml.org/lkml/2019/4/5/856

Please take a look.

regards,
  Ondrej Jirman

Icenowy Zheng (2):
  net: stmmac: sun8i: add support for Allwinner H6 EMAC
  net: stmmac: sun8i: force select external PHY when no internal one

Ondrej Jirman (10):
  arm64: dts: allwinner: h6: Add Orange Pi 3 DTS
  drm: sun4i: Add support for enabling DDC I2C bus to dw_hdmi glue
  dt-bindings: display: sun4i-drm: Add DDC power supply
  arm64: dts: allwinner: orange-pi-3: Enable HDMI output
  arm64: dts: allwinner: orange-pi-3: Enable ethernet
  arm64: dts: allwinner: h6: Add MMC1 pins
  pinctrl: sunxi: Prepare for alternative bias voltage setting methods
  pinctrl: sunxi: Support I/O bias voltage setting on H6
  brcmfmac: Loading the correct firmware for brcm43456
  arm64: dts: allwinner: orange-pi-3: Enable WiFi

 .../bindings/display/sunxi/sun4i-drm.txt      |   1 +
 arch/arm64/boot/dts/allwinner/Makefile        |   1 +
 .../dts/allwinner/sun50i-h6-orangepi-3.dts    | 339 ++++++++++++++++++
 arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi  |   9 +
 drivers/gpu/drm/sun4i/sun8i_dw_hdmi.c         |  17 +-
 drivers/gpu/drm/sun4i/sun8i_dw_hdmi.h         |   1 +
 .../net/ethernet/stmicro/stmmac/dwmac-sun8i.c |  22 ++
 .../broadcom/brcm80211/brcmfmac/sdio.c        |   4 +-
 drivers/pinctrl/sunxi/pinctrl-sun50i-h6.c     |   1 +
 drivers/pinctrl/sunxi/pinctrl-sun9i-a80.c     |   2 +-
 drivers/pinctrl/sunxi/pinctrl-sunxi.c         |  50 ++-
 drivers/pinctrl/sunxi/pinctrl-sunxi.h         |   7 +-
 12 files changed, 433 insertions(+), 21 deletions(-)
 create mode 100644 arch/arm64/boot/dts/allwinner/sun50i-h6-orangepi-3.dts

Comments

Sergei Shtylyov April 6, 2019, 10:24 a.m. UTC | #1
Hello!

On 06.04.2019 2:45, megous@megous.com wrote:

> From: Icenowy Zheng <icenowy@aosc.io>
> 
> The PHY selection bit also exists on SoCs without an internal PHY; if
> it's set to 1 (internal PHY) then the MAC will not make use of any PHY.
> 
> This problem appears when adapting for H6, which has no real internal
> PHY (the "internal PHY" on H6 is not on-die, but on a co-packaged AC200
> chip, via RMII interface at GPIO bank A), but the PHY selection bit is
> set.
> 
> Force the PHY selection bit to 0 when no external PHY to select this
> problem.

    "Select this problem" sound weird...

> Signed-off-by: Icenowy Zheng <icenowy@aosc.io>
> ---
>   drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c | 5 +++++
>   1 file changed, 5 insertions(+)
> 
> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
> index e3e3dc44b33b..bd340e77b2ea 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
> @@ -908,6 +908,11 @@ static int sun8i_dwmac_set_syscon(struct stmmac_priv *priv)
>   		 * address. No need to mask it again.
>   		 */
>   		reg |= 1 << H3_EPHY_ADDR_SHIFT;
> +	} else {
> +		/* For SoCs without internal PHY the PHY selection bit should be
> +		 * set to 0 (external PHY).
> +		 */
> +		reg &= ~(H3_EPHY_SELECT);
    No () should be needed here, add () around the macro body if still needed.

[...]

MBR, Sergei
Clément Péron April 7, 2019, 1:36 p.m. UTC | #2
Hi,

On Sat, 6 Apr 2019 at 01:45, megous via linux-sunxi
<linux-sunxi@googlegroups.com> wrote:
>
> From: Ondrej Jirman <megous@megous.com>
>
> This series implements support for Xunlong Orange Pi 3 board.

OrangePi 3 Lite2 and One Plus boards support has already been merged.
The support is not complete but you should rebase your patches on top
of sunxi/for-next

https://git.kernel.org/pub/scm/linux/kernel/git/sunxi/linux.git/tree/arch/arm64/boot/dts/allwinner/sun50i-h6-orangepi.dtsi?h=sunxi/for-next

Regards,
Clement

>
> Unfortunately, this board needs some small driver patches, so I have
> split the boards DT patch into chunks that require patches for drivers
> in various subsystems:
>
> - Basic DT for the board  (patch 1)
> - HDMI support            (patches 2, 3, 4)
> - Ethernet support        (patches 5, 6, 7)
> - WiFi support            (patches 8, 9, 10, 11, 12)
>
> This patch is also needed to not get segfault on boot:
>   https://lkml.org/lkml/2019/4/5/856
>
> Please take a look.
>
> regards,
>   Ondrej Jirman
>
> Icenowy Zheng (2):
>   net: stmmac: sun8i: add support for Allwinner H6 EMAC
>   net: stmmac: sun8i: force select external PHY when no internal one
>
> Ondrej Jirman (10):
>   arm64: dts: allwinner: h6: Add Orange Pi 3 DTS
>   drm: sun4i: Add support for enabling DDC I2C bus to dw_hdmi glue
>   dt-bindings: display: sun4i-drm: Add DDC power supply
>   arm64: dts: allwinner: orange-pi-3: Enable HDMI output
>   arm64: dts: allwinner: orange-pi-3: Enable ethernet
>   arm64: dts: allwinner: h6: Add MMC1 pins
>   pinctrl: sunxi: Prepare for alternative bias voltage setting methods
>   pinctrl: sunxi: Support I/O bias voltage setting on H6
>   brcmfmac: Loading the correct firmware for brcm43456
>   arm64: dts: allwinner: orange-pi-3: Enable WiFi
>
>  .../bindings/display/sunxi/sun4i-drm.txt      |   1 +
>  arch/arm64/boot/dts/allwinner/Makefile        |   1 +
>  .../dts/allwinner/sun50i-h6-orangepi-3.dts    | 339 ++++++++++++++++++
>  arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi  |   9 +
>  drivers/gpu/drm/sun4i/sun8i_dw_hdmi.c         |  17 +-
>  drivers/gpu/drm/sun4i/sun8i_dw_hdmi.h         |   1 +
>  .../net/ethernet/stmicro/stmmac/dwmac-sun8i.c |  22 ++
>  .../broadcom/brcm80211/brcmfmac/sdio.c        |   4 +-
>  drivers/pinctrl/sunxi/pinctrl-sun50i-h6.c     |   1 +
>  drivers/pinctrl/sunxi/pinctrl-sun9i-a80.c     |   2 +-
>  drivers/pinctrl/sunxi/pinctrl-sunxi.c         |  50 ++-
>  drivers/pinctrl/sunxi/pinctrl-sunxi.h         |   7 +-
>  12 files changed, 433 insertions(+), 21 deletions(-)
>  create mode 100644 arch/arm64/boot/dts/allwinner/sun50i-h6-orangepi-3.dts
>
> --
> 2.21.0
>
> --
> You received this message because you are subscribed to the Google Groups "linux-sunxi" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscribe@googlegroups.com.
> For more options, visit https://groups.google.com/d/optout.
Ondřej Jirman April 7, 2019, 2:32 p.m. UTC | #3
On Sun, Apr 07, 2019 at 03:36:21PM +0200, Clément Péron wrote:
> Hi,
> 
> On Sat, 6 Apr 2019 at 01:45, megous via linux-sunxi
> <linux-sunxi@googlegroups.com> wrote:
> >
> > From: Ondrej Jirman <megous@megous.com>
> >
> > This series implements support for Xunlong Orange Pi 3 board.
> 
> OrangePi 3 Lite2 and One Plus boards support has already been merged.
> The support is not complete but you should rebase your patches on top
> of sunxi/for-next

Hi,

OrangePi 3 is somewhat different from these two boards (mostly it has a differnt
power tree). It doesn't use the AXP regulators that are defined in the
sun50i-h6-orangepi.dtsi in the same way.

For example:

- bldo3 (is turned always on in sun50i-h6-orangepi.dtsi but unused for opi3)
- cldo2 and cldo3 are unused on opi3 and have nothing to do with WiFi
- aldo3 is not for dram
- bldo1 on the other hand is for dram on opi3
- some other regulators are used for different/more functions and thus
  named differntly
- USB id-det pin is differnt
- ...

OrangePi 3 is not a superset of what is defined in sun50i-h6-orangepi.dtsi.

So to base Orange Pi 3 dts on top of existing sun50i-h6-orangepi.dtsi I'd have
to first move some things out of the base dtsi to the OrangePi Lite2 and One
Plus board dts files, in order to have sun50i-h6-orangepi.dtsi only describe HW
that is *really* shared by these 2 boards and Orange Pi 3.

If I do that, I'd undefine all the axp805 regulator nodes and move the
configurations to each of the 3 board files. That will probably end up being
the least confusing and most maintainable. See axp81x.dtsi lines 86-144 for
what I mean.

What do you think? Is this acceptable to everyone?

regards,
  o.

> https://git.kernel.org/pub/scm/linux/kernel/git/sunxi/linux.git/tree/arch/arm64/boot/dts/allwinner/sun50i-h6-orangepi.dtsi?h=sunxi/for-next
> 
> Regards,
> Clement
> 
> >
> > Unfortunately, this board needs some small driver patches, so I have
> > split the boards DT patch into chunks that require patches for drivers
> > in various subsystems:
> >
> > - Basic DT for the board  (patch 1)
> > - HDMI support            (patches 2, 3, 4)
> > - Ethernet support        (patches 5, 6, 7)
> > - WiFi support            (patches 8, 9, 10, 11, 12)
> >
> > This patch is also needed to not get segfault on boot:
> >   https://lkml.org/lkml/2019/4/5/856
> >
> > Please take a look.
> >
> > regards,
> >   Ondrej Jirman
> >
> > Icenowy Zheng (2):
> >   net: stmmac: sun8i: add support for Allwinner H6 EMAC
> >   net: stmmac: sun8i: force select external PHY when no internal one
> >
> > Ondrej Jirman (10):
> >   arm64: dts: allwinner: h6: Add Orange Pi 3 DTS
> >   drm: sun4i: Add support for enabling DDC I2C bus to dw_hdmi glue
> >   dt-bindings: display: sun4i-drm: Add DDC power supply
> >   arm64: dts: allwinner: orange-pi-3: Enable HDMI output
> >   arm64: dts: allwinner: orange-pi-3: Enable ethernet
> >   arm64: dts: allwinner: h6: Add MMC1 pins
> >   pinctrl: sunxi: Prepare for alternative bias voltage setting methods
> >   pinctrl: sunxi: Support I/O bias voltage setting on H6
> >   brcmfmac: Loading the correct firmware for brcm43456
> >   arm64: dts: allwinner: orange-pi-3: Enable WiFi
> >
> >  .../bindings/display/sunxi/sun4i-drm.txt      |   1 +
> >  arch/arm64/boot/dts/allwinner/Makefile        |   1 +
> >  .../dts/allwinner/sun50i-h6-orangepi-3.dts    | 339 ++++++++++++++++++
> >  arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi  |   9 +
> >  drivers/gpu/drm/sun4i/sun8i_dw_hdmi.c         |  17 +-
> >  drivers/gpu/drm/sun4i/sun8i_dw_hdmi.h         |   1 +
> >  .../net/ethernet/stmicro/stmmac/dwmac-sun8i.c |  22 ++
> >  .../broadcom/brcm80211/brcmfmac/sdio.c        |   4 +-
> >  drivers/pinctrl/sunxi/pinctrl-sun50i-h6.c     |   1 +
> >  drivers/pinctrl/sunxi/pinctrl-sun9i-a80.c     |   2 +-
> >  drivers/pinctrl/sunxi/pinctrl-sunxi.c         |  50 ++-
> >  drivers/pinctrl/sunxi/pinctrl-sunxi.h         |   7 +-
> >  12 files changed, 433 insertions(+), 21 deletions(-)
> >  create mode 100644 arch/arm64/boot/dts/allwinner/sun50i-h6-orangepi-3.dts
> >
> > --
> > 2.21.0
> >
> > --
> > You received this message because you are subscribed to the Google Groups "linux-sunxi" group.
> > To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscribe@googlegroups.com.
> > For more options, visit https://groups.google.com/d/optout.
Clément Péron April 7, 2019, 3:08 p.m. UTC | #4
Hi,

On Sun, 7 Apr 2019 at 16:32, Ondřej Jirman <megous@megous.com> wrote:
>
> On Sun, Apr 07, 2019 at 03:36:21PM +0200, Clément Péron wrote:
> > Hi,
> >
> > On Sat, 6 Apr 2019 at 01:45, megous via linux-sunxi
> > <linux-sunxi@googlegroups.com> wrote:
> > >
> > > From: Ondrej Jirman <megous@megous.com>
> > >
> > > This series implements support for Xunlong Orange Pi 3 board.
> >
> > OrangePi 3 Lite2 and One Plus boards support has already been merged.
> > The support is not complete but you should rebase your patches on top
> > of sunxi/for-next
>
> Hi,
>
> OrangePi 3 is somewhat different from these two boards (mostly it has a differnt
> power tree). It doesn't use the AXP regulators that are defined in the
> sun50i-h6-orangepi.dtsi in the same way.
>
> For example:
>
> - bldo3 (is turned always on in sun50i-h6-orangepi.dtsi but unused for opi3)
> - cldo2 and cldo3 are unused on opi3 and have nothing to do with WiFi
> - aldo3 is not for dram
> - bldo1 on the other hand is for dram on opi3
> - some other regulators are used for different/more functions and thus
>   named differntly
> - USB id-det pin is differnt
> - ...
>
> OrangePi 3 is not a superset of what is defined in sun50i-h6-orangepi.dtsi.
>
> So to base Orange Pi 3 dts on top of existing sun50i-h6-orangepi.dtsi I'd have
> to first move some things out of the base dtsi to the OrangePi Lite2 and One
> Plus board dts files, in order to have sun50i-h6-orangepi.dtsi only describe HW
> that is *really* shared by these 2 boards and Orange Pi 3.
>
> If I do that, I'd undefine all the axp805 regulator nodes and move the
> configurations to each of the 3 board files. That will probably end up being
> the least confusing and most maintainable. See axp81x.dtsi lines 86-144 for
> what I mean.
>
> What do you think? Is this acceptable to everyone?

Indeed it seems to be a totally different board, not as much in common
as I thought with Lite2 and One Plus.

You should also add your board in Documentation
devicetree/bindings/arm/sunxi.yaml

Regards,
Clement



>
> regards,
>   o.
>
> > https://git.kernel.org/pub/scm/linux/kernel/git/sunxi/linux.git/tree/arch/arm64/boot/dts/allwinner/sun50i-h6-orangepi.dtsi?h=sunxi/for-next
> >
> > Regards,
> > Clement
> >
> > >
> > > Unfortunately, this board needs some small driver patches, so I have
> > > split the boards DT patch into chunks that require patches for drivers
> > > in various subsystems:
> > >
> > > - Basic DT for the board  (patch 1)
> > > - HDMI support            (patches 2, 3, 4)
> > > - Ethernet support        (patches 5, 6, 7)
> > > - WiFi support            (patches 8, 9, 10, 11, 12)
> > >
> > > This patch is also needed to not get segfault on boot:
> > >   https://lkml.org/lkml/2019/4/5/856
> > >
> > > Please take a look.
> > >
> > > regards,
> > >   Ondrej Jirman
> > >
> > > Icenowy Zheng (2):
> > >   net: stmmac: sun8i: add support for Allwinner H6 EMAC
> > >   net: stmmac: sun8i: force select external PHY when no internal one
> > >
> > > Ondrej Jirman (10):
> > >   arm64: dts: allwinner: h6: Add Orange Pi 3 DTS
> > >   drm: sun4i: Add support for enabling DDC I2C bus to dw_hdmi glue
> > >   dt-bindings: display: sun4i-drm: Add DDC power supply
> > >   arm64: dts: allwinner: orange-pi-3: Enable HDMI output
> > >   arm64: dts: allwinner: orange-pi-3: Enable ethernet
> > >   arm64: dts: allwinner: h6: Add MMC1 pins
> > >   pinctrl: sunxi: Prepare for alternative bias voltage setting methods
> > >   pinctrl: sunxi: Support I/O bias voltage setting on H6
> > >   brcmfmac: Loading the correct firmware for brcm43456
> > >   arm64: dts: allwinner: orange-pi-3: Enable WiFi
> > >
> > >  .../bindings/display/sunxi/sun4i-drm.txt      |   1 +
> > >  arch/arm64/boot/dts/allwinner/Makefile        |   1 +
> > >  .../dts/allwinner/sun50i-h6-orangepi-3.dts    | 339 ++++++++++++++++++
> > >  arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi  |   9 +
> > >  drivers/gpu/drm/sun4i/sun8i_dw_hdmi.c         |  17 +-
> > >  drivers/gpu/drm/sun4i/sun8i_dw_hdmi.h         |   1 +
> > >  .../net/ethernet/stmicro/stmmac/dwmac-sun8i.c |  22 ++
> > >  .../broadcom/brcm80211/brcmfmac/sdio.c        |   4 +-
> > >  drivers/pinctrl/sunxi/pinctrl-sun50i-h6.c     |   1 +
> > >  drivers/pinctrl/sunxi/pinctrl-sun9i-a80.c     |   2 +-
> > >  drivers/pinctrl/sunxi/pinctrl-sunxi.c         |  50 ++-
> > >  drivers/pinctrl/sunxi/pinctrl-sunxi.h         |   7 +-
> > >  12 files changed, 433 insertions(+), 21 deletions(-)
> > >  create mode 100644 arch/arm64/boot/dts/allwinner/sun50i-h6-orangepi-3.dts
> > >
> > > --
> > > 2.21.0
> > >
> > > --
> > > You received this message because you are subscribed to the Google Groups "linux-sunxi" group.
> > > To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscribe@googlegroups.com.
> > > For more options, visit https://groups.google.com/d/optout.
Clément Péron April 7, 2019, 3:31 p.m. UTC | #5
Hi,

On Sat, 6 Apr 2019 at 01:45, megous via linux-sunxi
<linux-sunxi@googlegroups.com> wrote:
>
> From: Ondrej Jirman <megous@megous.com>
>
> Orange Pi 3 has AP6256 WiFi/BT module. WiFi part of the module is
> called bcm43356 and can be used with the brcmfmac driver. The module
> is powered by the two always on regulators (not AXP805).
>
> WiFi uses a PG port with 1.8V voltage level signals. SoC needs to be
> configured so that it sets up an 1.8V input bias on this port. This is
> done by the pio driver by reading the vcc-pg-supply voltage.
>
> You'll need a fw_bcm43456c5_ag.bin firmware file and nvram.txt
> configuration that can be found in the Xulongs's repository for H6:
>
> https://github.com/orangepi-xunlong/OrangePiH6_external/tree/master/ap6256
>
> Mainline brcmfmac driver expects the firmware and nvram at the
> following paths relative to the firmware directory:
>
>   brcm/brcmfmac43456-sdio.bin
>   brcm/brcmfmac43456-sdio.txt
>
> Signed-off-by: Ondrej Jirman <megous@megous.com>
> ---
>  .../dts/allwinner/sun50i-h6-orangepi-3.dts    | 48 +++++++++++++++++++
>  1 file changed, 48 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/allwinner/sun50i-h6-orangepi-3.dts b/arch/arm64/boot/dts/allwinner/sun50i-h6-orangepi-3.dts
> index 5270142527f5..6a201829bb62 100644
> --- a/arch/arm64/boot/dts/allwinner/sun50i-h6-orangepi-3.dts
> +++ b/arch/arm64/boot/dts/allwinner/sun50i-h6-orangepi-3.dts
> @@ -66,6 +66,26 @@
>                 regulator-always-on;
>         };
>
> +       reg_vcc33_wifi: vcc33-wifi {
> +               /* Always on 3.3V regulator for WiFi and BT */
> +               compatible = "regulator-fixed";
> +               regulator-name = "vcc33-wifi";
> +               regulator-min-microvolt = <3300000>;
> +               regulator-max-microvolt = <3300000>;
> +               regulator-always-on;
> +                vin-supply = <&reg_vcc5v>;
> +       };
> +
> +       reg_vcc_wifi_io: vcc-wifi-io {
> +               /* Always on 1.8V/300mA regulator for WiFi and BT IO */
> +               compatible = "regulator-fixed";
> +               regulator-name = "vcc-wifi-io";
> +               regulator-min-microvolt = <1800000>;
> +               regulator-max-microvolt = <1800000>;
> +               regulator-always-on;
> +                vin-supply = <&reg_vcc33_wifi>;
> +       };
> +
>         /*
>          * The board uses 2.5V RGMII signalling. Power sequence
>          * to enable the phy is to enable GMAC-2V5 and GMAC-3V3 (aldo2)
> @@ -86,6 +106,14 @@
>                   */
>                  vin-supply = <&reg_aldo2>; /* GMAC-3V3 */
>          };
> +
> +       wifi_pwrseq: wifi_pwrseq {
> +               compatible = "mmc-pwrseq-simple";
> +               clocks = <&rtc 1>;

Maybe I missed something, but the RTC in H6 is not yet available :
https://lkml.org/lkml/2018/10/31/822

Regards,
Clement

> +               clock-names = "ext_clock";
> +               reset-gpios = <&r_pio 1 3 GPIO_ACTIVE_LOW>; /* PM3 */
> +               post-power-on-delay-ms = <200>;
> +       };
>  };
>
>  &cpu0 {
> @@ -146,6 +174,25 @@
>         status = "okay";
>  };
>
> +&mmc1 {
> +       pinctrl-names = "default";
> +       pinctrl-0 = <&mmc1_pins>;
> +       vmmc-supply = <&reg_vcc33_wifi>;
> +       vqmmc-supply = <&reg_vcc_wifi_io>;
> +       mmc-pwrseq = <&wifi_pwrseq>;
> +       bus-width = <4>;
> +       non-removable;
> +       status = "okay";
> +
> +       brcm: sdio-wifi@1 {
> +               reg = <1>;
> +               compatible = "brcm,bcm4329-fmac";
> +               interrupt-parent = <&r_pio>;
> +               interrupts = <1 0 IRQ_TYPE_LEVEL_LOW>; /* PM0 */
> +               interrupt-names = "host-wake";
> +       };
> +};
> +
>  &ohci0 {
>         status = "okay";
>  };
> @@ -157,6 +204,7 @@
>  &pio {
>         vcc-pc-supply = <&reg_bldo2>;
>         vcc-pd-supply = <&reg_cldo1>;
> +       vcc-pg-supply = <&reg_vcc_wifi_io>;
>  };
>
>  &r_i2c {
> --
> 2.21.0
>
> --
> You received this message because you are subscribed to the Google Groups "linux-sunxi" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscribe@googlegroups.com.
> For more options, visit https://groups.google.com/d/optout.
Ondřej Jirman April 7, 2019, 4:15 p.m. UTC | #6
On Sun, Apr 07, 2019 at 05:31:52PM +0200, Clément Péron wrote:
> Hi,
> 
> On Sat, 6 Apr 2019 at 01:45, megous via linux-sunxi
> <linux-sunxi@googlegroups.com> wrote:
> >
> > From: Ondrej Jirman <megous@megous.com>
> >
> > Orange Pi 3 has AP6256 WiFi/BT module. WiFi part of the module is
> > called bcm43356 and can be used with the brcmfmac driver. The module
> > is powered by the two always on regulators (not AXP805).
> >
> > WiFi uses a PG port with 1.8V voltage level signals. SoC needs to be
> > configured so that it sets up an 1.8V input bias on this port. This is
> > done by the pio driver by reading the vcc-pg-supply voltage.
> >
> > You'll need a fw_bcm43456c5_ag.bin firmware file and nvram.txt
> > configuration that can be found in the Xulongs's repository for H6:
> >
> > https://github.com/orangepi-xunlong/OrangePiH6_external/tree/master/ap6256
> >
> > Mainline brcmfmac driver expects the firmware and nvram at the
> > following paths relative to the firmware directory:
> >
> >   brcm/brcmfmac43456-sdio.bin
> >   brcm/brcmfmac43456-sdio.txt
> >
> > Signed-off-by: Ondrej Jirman <megous@megous.com>
> > ---
> >  .../dts/allwinner/sun50i-h6-orangepi-3.dts    | 48 +++++++++++++++++++
> >  1 file changed, 48 insertions(+)
> >
> > diff --git a/arch/arm64/boot/dts/allwinner/sun50i-h6-orangepi-3.dts b/arch/arm64/boot/dts/allwinner/sun50i-h6-orangepi-3.dts
> > index 5270142527f5..6a201829bb62 100644
> > --- a/arch/arm64/boot/dts/allwinner/sun50i-h6-orangepi-3.dts
> > +++ b/arch/arm64/boot/dts/allwinner/sun50i-h6-orangepi-3.dts
> > @@ -66,6 +66,26 @@
> >                 regulator-always-on;
> >         };
> >
> > +       reg_vcc33_wifi: vcc33-wifi {
> > +               /* Always on 3.3V regulator for WiFi and BT */
> > +               compatible = "regulator-fixed";
> > +               regulator-name = "vcc33-wifi";
> > +               regulator-min-microvolt = <3300000>;
> > +               regulator-max-microvolt = <3300000>;
> > +               regulator-always-on;
> > +                vin-supply = <&reg_vcc5v>;
> > +       };
> > +
> > +       reg_vcc_wifi_io: vcc-wifi-io {
> > +               /* Always on 1.8V/300mA regulator for WiFi and BT IO */
> > +               compatible = "regulator-fixed";
> > +               regulator-name = "vcc-wifi-io";
> > +               regulator-min-microvolt = <1800000>;
> > +               regulator-max-microvolt = <1800000>;
> > +               regulator-always-on;
> > +                vin-supply = <&reg_vcc33_wifi>;
> > +       };
> > +
> >         /*
> >          * The board uses 2.5V RGMII signalling. Power sequence
> >          * to enable the phy is to enable GMAC-2V5 and GMAC-3V3 (aldo2)
> > @@ -86,6 +106,14 @@
> >                   */
> >                  vin-supply = <&reg_aldo2>; /* GMAC-3V3 */
> >          };
> > +
> > +       wifi_pwrseq: wifi_pwrseq {
> > +               compatible = "mmc-pwrseq-simple";
> > +               clocks = <&rtc 1>;
> 
> Maybe I missed something, but the RTC in H6 is not yet available :
> https://lkml.org/lkml/2018/10/31/822

You're right. I'm using an out-of-tree patch for that and didn't notice the
dependency. I guess, WiFi DTS patch can be ignored for now.

thanks,
	o.

> Regards,
> Clement
> 
> > +               clock-names = "ext_clock";
> > +               reset-gpios = <&r_pio 1 3 GPIO_ACTIVE_LOW>; /* PM3 */
> > +               post-power-on-delay-ms = <200>;
> > +       };
> >  };
> >
> >  &cpu0 {
> > @@ -146,6 +174,25 @@
> >         status = "okay";
> >  };
> >
> > +&mmc1 {
> > +       pinctrl-names = "default";
> > +       pinctrl-0 = <&mmc1_pins>;
> > +       vmmc-supply = <&reg_vcc33_wifi>;
> > +       vqmmc-supply = <&reg_vcc_wifi_io>;
> > +       mmc-pwrseq = <&wifi_pwrseq>;
> > +       bus-width = <4>;
> > +       non-removable;
> > +       status = "okay";
> > +
> > +       brcm: sdio-wifi@1 {
> > +               reg = <1>;
> > +               compatible = "brcm,bcm4329-fmac";
> > +               interrupt-parent = <&r_pio>;
> > +               interrupts = <1 0 IRQ_TYPE_LEVEL_LOW>; /* PM0 */
> > +               interrupt-names = "host-wake";
> > +       };
> > +};
> > +
> >  &ohci0 {
> >         status = "okay";
> >  };
> > @@ -157,6 +204,7 @@
> >  &pio {
> >         vcc-pc-supply = <&reg_bldo2>;
> >         vcc-pd-supply = <&reg_cldo1>;
> > +       vcc-pg-supply = <&reg_vcc_wifi_io>;
> >  };
> >
> >  &r_i2c {
> > --
> > 2.21.0
> >
> > --
> > You received this message because you are subscribed to the Google Groups "linux-sunxi" group.
> > To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscribe@googlegroups.com.
> > For more options, visit https://groups.google.com/d/optout.
Ondřej Jirman April 8, 2019, 1:31 a.m. UTC | #7
On Sat, Apr 06, 2019 at 01:45:12AM +0200, verejna wrote:
> From: Ondrej Jirman <megous@megous.com>
> 
> H6 SoC has a "pio group withstand voltage mode" register (datasheet
> description), that needs to be used to select either 1.8V or 3.3V
> I/O mode, based on what voltage is powering the respective pin
> banks and is thus used for I/O signals.
> 
> Add support for configuring this register according to the voltage
> of the pin bank regulator (if enabled).
> 
> This is similar to the support for I/O bias voltage setting patch
> for A80 and the same concerns apply. (see commit 402bfb3c135213dc
> Support I/O bias voltage setting on A80).
> 
> Signed-off-by: Ondrej Jirman <megous@megous.com>
> ---
>  drivers/pinctrl/sunxi/pinctrl-sun50i-h6.c |  1 +
>  drivers/pinctrl/sunxi/pinctrl-sunxi.c     | 14 ++++++++++++++
>  drivers/pinctrl/sunxi/pinctrl-sunxi.h     |  3 +++
>  3 files changed, 18 insertions(+)
> 
> diff --git a/drivers/pinctrl/sunxi/pinctrl-sun50i-h6.c b/drivers/pinctrl/sunxi/pinctrl-sun50i-h6.c
> index ef4268cc6227..30b1befa8ed8 100644
> --- a/drivers/pinctrl/sunxi/pinctrl-sun50i-h6.c
> +++ b/drivers/pinctrl/sunxi/pinctrl-sun50i-h6.c
> @@ -591,6 +591,7 @@ static const struct sunxi_pinctrl_desc h6_pinctrl_data = {
>  	.irq_banks = 4,
>  	.irq_bank_map = h6_irq_bank_map,
>  	.irq_read_needs_mux = true,
> +	.io_bias_cfg_variant = IO_BIAS_CFG_V2,
>  };
>  
>  static int h6_pinctrl_probe(struct platform_device *pdev)
> diff --git a/drivers/pinctrl/sunxi/pinctrl-sunxi.c b/drivers/pinctrl/sunxi/pinctrl-sunxi.c
> index 9f329fec77cf..59a4ed396d92 100644
> --- a/drivers/pinctrl/sunxi/pinctrl-sunxi.c
> +++ b/drivers/pinctrl/sunxi/pinctrl-sunxi.c
> @@ -607,6 +607,8 @@ static int sunxi_pinctrl_set_io_bias_cfg(struct sunxi_pinctrl *pctl,
>  					 unsigned pin,
>  					 struct regulator *supply)
>  {
> +	unsigned short bank = pin / PINS_PER_BANK;
> +	unsigned long flags;
>  	u32 val, reg;
>  	int uV;
>  
> @@ -639,6 +641,18 @@ static int sunxi_pinctrl_set_io_bias_cfg(struct sunxi_pinctrl *pctl,
>  		reg = readl(pctl->membase + sunxi_grp_config_reg(pin));
>  		reg &= ~IO_BIAS_MASK;
>  		writel(reg | val, pctl->membase + sunxi_grp_config_reg(pin));
> +	} else if (pctl->desc->io_bias_cfg_variant == IO_BIAS_CFG_V2) {
> +		val = uV <= 1800000 ? 1 : 0;
> +
> +		dev_info(pctl->dev,
> +			 "Setting voltage bias to %sV on bank P%c\n",
> +			 val ? "1.8" : "3.3", 'A' + bank);

I'll drop this logging in v2. I forgot it here, after testing the patch.

	o.

> +		raw_spin_lock_irqsave(&pctl->lock, flags);
> +		reg = readl(pctl->membase + PIO_POW_MOD_SEL_REG);
> +		reg &= ~(1 << bank);
> +		writel(reg | val << bank, pctl->membase + PIO_POW_MOD_SEL_REG);
> +		raw_spin_unlock_irqrestore(&pctl->lock, flags);
>  	}
>  
>  	return 0;
> diff --git a/drivers/pinctrl/sunxi/pinctrl-sunxi.h b/drivers/pinctrl/sunxi/pinctrl-sunxi.h
> index 476772f91dba..3a66376f141b 100644
> --- a/drivers/pinctrl/sunxi/pinctrl-sunxi.h
> +++ b/drivers/pinctrl/sunxi/pinctrl-sunxi.h
> @@ -95,7 +95,10 @@
>  #define PINCTRL_SUN7I_A20	BIT(7)
>  #define PINCTRL_SUN8I_R40	BIT(8)
>  
> +#define PIO_POW_MOD_SEL_REG	0x340
> +
>  #define IO_BIAS_CFG_V1		1
> +#define IO_BIAS_CFG_V2		2
>  
>  struct sunxi_desc_function {
>  	unsigned long	variant;
> -- 
> 2.21.0
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Jagan Teki April 8, 2019, 6:01 a.m. UTC | #8
On Sun, Apr 7, 2019 at 8:02 PM 'Ondřej Jirman' via linux-sunxi
<linux-sunxi@googlegroups.com> wrote:
>
> On Sun, Apr 07, 2019 at 03:36:21PM +0200, Clément Péron wrote:
> > Hi,
> >
> > On Sat, 6 Apr 2019 at 01:45, megous via linux-sunxi
> > <linux-sunxi@googlegroups.com> wrote:
> > >
> > > From: Ondrej Jirman <megous@megous.com>
> > >
> > > This series implements support for Xunlong Orange Pi 3 board.
> >
> > OrangePi 3 Lite2 and One Plus boards support has already been merged.
> > The support is not complete but you should rebase your patches on top
> > of sunxi/for-next
>
> Hi,
>
> OrangePi 3 is somewhat different from these two boards (mostly it has a differnt
> power tree). It doesn't use the AXP regulators that are defined in the
> sun50i-h6-orangepi.dtsi in the same way.
>
> For example:
>
> - bldo3 (is turned always on in sun50i-h6-orangepi.dtsi but unused for opi3)
> - cldo2 and cldo3 are unused on opi3 and have nothing to do with WiFi
> - aldo3 is not for dram
> - bldo1 on the other hand is for dram on opi3
> - some other regulators are used for different/more functions and thus
>   named differntly
> - USB id-det pin is differnt
> - ...

Based on my communication with OrangePI, OPI-3 has PCIE, 4 USB-3.0
ports and AV are the key differences and rest seems to be similar. but
if we have a diff or unused regulators may be we can't enable them in
dtsi (I never looked that close as of now)
Jagan Teki April 8, 2019, 6:11 a.m. UTC | #9
On Sat, Apr 6, 2019 at 5:15 AM <megous@megous.com> wrote:
>
> From: Ondrej Jirman <megous@megous.com>
>
> Orange Pi 3 has two regulators that power the Realtek RTL8211E.
> According to the phy datasheet, both regulators need to be enabled
> at the same time, but we can only specify a single phy-supply in
> the DT.
>
> This can be achieved by making one regulator depedning on the
> other via vin-supply. While it's not a technically correct
> description of the hardware, it achieves the purpose.
>
> All values of RX/TX delay were tested exhaustively and a middle
> one of the working values was chosen.
>
> Signed-off-by: Ondrej Jirman <megous@megous.com>
> ---
>  .../dts/allwinner/sun50i-h6-orangepi-3.dts    | 44 +++++++++++++++++++
>  1 file changed, 44 insertions(+)

This was in ML[1], I thought this change would already merged. I
remember Chen-Yu applied and revert due to emac node not mainlined at
that time.

[1] https://patchwork.kernel.org/patch/10558281/
Maxime Ripard April 8, 2019, 7:23 a.m. UTC | #10
On Sat, Apr 06, 2019 at 01:45:04AM +0200, megous@megous.com wrote:
> From: Ondrej Jirman <megous@megous.com>
>
> Orange Pi 3 board requires enabling DDC I2C bus via some GPIO connected
> transistors, before it can be used. Model this as a power supply for DDC
> (via regulator framework).
>
> Signed-off-by: Ondrej Jirman <megous@megous.com>

The DDC bus itself is usually attached to the HDMI connector, so it
would make sense to make the supply also a property of the connector.

Maxime

--
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
Maxime Ripard April 8, 2019, 7:24 a.m. UTC | #11
On Sat, Apr 06, 2019 at 01:45:06AM +0200, megous@megous.com wrote:
> From: Ondrej Jirman <megous@megous.com>
>
> Orange Pi 3 has a DDC_CEC_EN signal connected to PH2, that enables
> the DDC I2C bus. Before EDID can be read, we need to pull PH2 high.
>
> Signed-off-by: Ondrej Jirman <megous@megous.com>
> ---
>  .../dts/allwinner/sun50i-h6-orangepi-3.dts    | 35 +++++++++++++++++++
>  1 file changed, 35 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/allwinner/sun50i-h6-orangepi-3.dts b/arch/arm64/boot/dts/allwinner/sun50i-h6-orangepi-3.dts
> index 7a2424fcaed7..644946749088 100644
> --- a/arch/arm64/boot/dts/allwinner/sun50i-h6-orangepi-3.dts
> +++ b/arch/arm64/boot/dts/allwinner/sun50i-h6-orangepi-3.dts
> @@ -21,6 +21,17 @@
>  		stdout-path = "serial0:115200n8";
>  	};
>
> +	connector {
> +		compatible = "hdmi-connector";
> +		type = "a";
> +
> +		port {
> +			hdmi_con_in: endpoint {
> +				remote-endpoint = <&hdmi_out_con>;
> +			};
> +		};
> +	};
> +
>  	leds {
>  		compatible = "gpio-leds";
>
> @@ -36,6 +47,15 @@
>  		};
>  	};
>
> +	reg_ddc: ddc-io {
> +                compatible = "regulator-fixed";
> +                regulator-name = "ddc-io";
> +                regulator-min-microvolt = <5000000>;
> +                regulator-max-microvolt = <5000000>;
> +                enable-active-high;
> +                gpio = <&pio 7 2 GPIO_ACTIVE_HIGH>; /* PH2 */
> +        };
> +

This is indented with spaces and generates checkpatch warnings.

Maxime

--
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
Maxime Ripard April 8, 2019, 7:25 a.m. UTC | #12
On Sat, Apr 06, 2019 at 01:45:07AM +0200, megous@megous.com wrote:
> From: Icenowy Zheng <icenowy@aosc.io>
>
> The EMAC on Allwinner H6 is just like the one on A64. The "internal PHY"
> on H6 is on a co-packaged AC200 chip, and it's not really internal (it's
> connected via RMII at PA GPIO bank).
>
> Add support for the Allwinner H6 EMAC in the dwmac-sun8i driver.
>
> Signed-off-by: Icenowy Zheng <icenowy@aosc.io>

This should have your Signed-off-by (just like all the other
subsequent patches from someone else you sent in this series).

Maxime

--
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
Chen-Yu Tsai April 8, 2019, 7:28 a.m. UTC | #13
On Mon, Apr 8, 2019 at 3:23 PM Maxime Ripard <maxime.ripard@bootlin.com> wrote:
>
> On Sat, Apr 06, 2019 at 01:45:04AM +0200, megous@megous.com wrote:
> > From: Ondrej Jirman <megous@megous.com>
> >
> > Orange Pi 3 board requires enabling DDC I2C bus via some GPIO connected
> > transistors, before it can be used. Model this as a power supply for DDC
> > (via regulator framework).
> >
> > Signed-off-by: Ondrej Jirman <megous@megous.com>
>
> The DDC bus itself is usually attached to the HDMI connector, so it
> would make sense to make the supply also a property of the connector.

I believe these are separate things. What this patch covers is power for
a voltage shifter between the SoC and HDMI DDC pins. The HDMI connector's
5V supply to power the remote DDC chip is something else. And on the
Orange Pi 3 they are indeed separate supplies.

ChenYu
Maxime Ripard April 8, 2019, 7:40 a.m. UTC | #14
On Sat, Apr 06, 2019 at 01:45:09AM +0200, megous@megous.com wrote:
> From: Ondrej Jirman <megous@megous.com>
>
> Orange Pi 3 has two regulators that power the Realtek RTL8211E.
> According to the phy datasheet, both regulators need to be enabled
> at the same time, but we can only specify a single phy-supply in
> the DT.
>
> This can be achieved by making one regulator depedning on the
> other via vin-supply. While it's not a technically correct
> description of the hardware, it achieves the purpose.
>
> All values of RX/TX delay were tested exhaustively and a middle
> one of the working values was chosen.
>
> Signed-off-by: Ondrej Jirman <megous@megous.com>
> ---
>  .../dts/allwinner/sun50i-h6-orangepi-3.dts    | 44 +++++++++++++++++++
>  1 file changed, 44 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/allwinner/sun50i-h6-orangepi-3.dts b/arch/arm64/boot/dts/allwinner/sun50i-h6-orangepi-3.dts
> index 644946749088..5270142527f5 100644
> --- a/arch/arm64/boot/dts/allwinner/sun50i-h6-orangepi-3.dts
> +++ b/arch/arm64/boot/dts/allwinner/sun50i-h6-orangepi-3.dts
> @@ -15,6 +15,7 @@
>
>  	aliases {
>  		serial0 = &uart0;
> +		ethernet0 = &emac;
>  	};
>
>  	chosen {
> @@ -64,6 +65,27 @@
>  		regulator-max-microvolt = <5000000>;
>  		regulator-always-on;
>  	};
> +
> +	/*
> +	 * The board uses 2.5V RGMII signalling. Power sequence
> +	 * to enable the phy is to enable GMAC-2V5 and GMAC-3V3 (aldo2)
> +	 * power rails at the same time and to wait 100ms.
> +	 */
> +	reg_gmac_2v5: gmac-2v5 {
> +                compatible = "regulator-fixed";
> +                regulator-name = "gmac-2v5";
> +                regulator-min-microvolt = <2500000>;
> +                regulator-max-microvolt = <2500000>;
> +                startup-delay-us = <100000>;
> +                enable-active-high;
> +                gpio = <&pio 3 6 GPIO_ACTIVE_HIGH>; /* PD6 */

Is enable-active-high still needed? It's redundant with the
GPIO_ACTIVE_HIGH flag.

The indentation is wrong here as well.

Maxime

--
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
Maxime Ripard April 8, 2019, 7:42 a.m. UTC | #15
On Sat, Apr 06, 2019 at 01:45:12AM +0200, megous@megous.com wrote:
> From: Ondrej Jirman <megous@megous.com>
>
> H6 SoC has a "pio group withstand voltage mode" register (datasheet
> description), that needs to be used to select either 1.8V or 3.3V
> I/O mode, based on what voltage is powering the respective pin
> banks and is thus used for I/O signals.
>
> Add support for configuring this register according to the voltage
> of the pin bank regulator (if enabled).
>
> This is similar to the support for I/O bias voltage setting patch
> for A80 and the same concerns apply. (see commit 402bfb3c135213dc
> Support I/O bias voltage setting on A80).
>
> Signed-off-by: Ondrej Jirman <megous@megous.com>
> ---
>  drivers/pinctrl/sunxi/pinctrl-sun50i-h6.c |  1 +
>  drivers/pinctrl/sunxi/pinctrl-sunxi.c     | 14 ++++++++++++++
>  drivers/pinctrl/sunxi/pinctrl-sunxi.h     |  3 +++
>  3 files changed, 18 insertions(+)
>
> diff --git a/drivers/pinctrl/sunxi/pinctrl-sun50i-h6.c b/drivers/pinctrl/sunxi/pinctrl-sun50i-h6.c
> index ef4268cc6227..30b1befa8ed8 100644
> --- a/drivers/pinctrl/sunxi/pinctrl-sun50i-h6.c
> +++ b/drivers/pinctrl/sunxi/pinctrl-sun50i-h6.c
> @@ -591,6 +591,7 @@ static const struct sunxi_pinctrl_desc h6_pinctrl_data = {
>  	.irq_banks = 4,
>  	.irq_bank_map = h6_irq_bank_map,
>  	.irq_read_needs_mux = true,
> +	.io_bias_cfg_variant = IO_BIAS_CFG_V2,
>  };
>
>  static int h6_pinctrl_probe(struct platform_device *pdev)
> diff --git a/drivers/pinctrl/sunxi/pinctrl-sunxi.c b/drivers/pinctrl/sunxi/pinctrl-sunxi.c
> index 9f329fec77cf..59a4ed396d92 100644
> --- a/drivers/pinctrl/sunxi/pinctrl-sunxi.c
> +++ b/drivers/pinctrl/sunxi/pinctrl-sunxi.c
> @@ -607,6 +607,8 @@ static int sunxi_pinctrl_set_io_bias_cfg(struct sunxi_pinctrl *pctl,
>  					 unsigned pin,
>  					 struct regulator *supply)
>  {
> +	unsigned short bank = pin / PINS_PER_BANK;
> +	unsigned long flags;
>  	u32 val, reg;
>  	int uV;
>
> @@ -639,6 +641,18 @@ static int sunxi_pinctrl_set_io_bias_cfg(struct sunxi_pinctrl *pctl,
>  		reg = readl(pctl->membase + sunxi_grp_config_reg(pin));
>  		reg &= ~IO_BIAS_MASK;
>  		writel(reg | val, pctl->membase + sunxi_grp_config_reg(pin));
> +	} else if (pctl->desc->io_bias_cfg_variant == IO_BIAS_CFG_V2) {
> +		val = uV <= 1800000 ? 1 : 0;
> +
> +		dev_info(pctl->dev,
> +			 "Setting voltage bias to %sV on bank P%c\n",
> +			 val ? "1.8" : "3.3", 'A' + bank);
> +
> +		raw_spin_lock_irqsave(&pctl->lock, flags);
> +		reg = readl(pctl->membase + PIO_POW_MOD_SEL_REG);
> +		reg &= ~(1 << bank);
> +		writel(reg | val << bank, pctl->membase + PIO_POW_MOD_SEL_REG);
> +		raw_spin_unlock_irqrestore(&pctl->lock, flags);
>  	}
>
>  	return 0;
> diff --git a/drivers/pinctrl/sunxi/pinctrl-sunxi.h b/drivers/pinctrl/sunxi/pinctrl-sunxi.h
> index 476772f91dba..3a66376f141b 100644
> --- a/drivers/pinctrl/sunxi/pinctrl-sunxi.h
> +++ b/drivers/pinctrl/sunxi/pinctrl-sunxi.h
> @@ -95,7 +95,10 @@
>  #define PINCTRL_SUN7I_A20	BIT(7)
>  #define PINCTRL_SUN8I_R40	BIT(8)
>
> +#define PIO_POW_MOD_SEL_REG	0x340
> +
>  #define IO_BIAS_CFG_V1		1
> +#define IO_BIAS_CFG_V2		2

Can you document what V1 and V2 means exactly?

Thanks!
Maxime

--
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
Maxime Ripard April 8, 2019, 7:43 a.m. UTC | #16
On Sat, Apr 06, 2019 at 01:45:10AM +0200, megous@megous.com wrote:
> From: Ondrej Jirman <megous@megous.com>
>
> ---
>  arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi | 9 +++++++++
>  1 file changed, 9 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi
> index 91fecab58836..dccad79da90c 100644
> --- a/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi
> +++ b/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi
> @@ -238,6 +238,15 @@
>  				bias-pull-up;
>  			};
>
> +

Extra line

> +			mmc1_pins: mmc1-pins {
> +				pins = "PG0", "PG1", "PG2", "PG3",
> +				       "PG4", "PG5";
> +				function = "mmc1";
> +				drive-strength = <30>;
> +				bias-pull-up;
> +			};
> +

Is that the only muxing option?

If so, then it should be assigned by default to mmc1

Thanks!
Maxime

--
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
Maxime Ripard April 8, 2019, 7:46 a.m. UTC | #17
On Sat, Apr 06, 2019 at 01:45:03AM +0200, megous@megous.com wrote:
> From: Ondrej Jirman <megous@megous.com>
>
> Orange Pi 3 is a H6 based SBC made by Xulong, released in
> January 2019. It has the following features:
>
> - Allwinner H6 quad-core 64-bit ARM Cortex-A53
> - GPU Mali-T720
> - 1GB or 2GB LPDDR3 RAM
> - AXP805 PMIC
> - AP6256 Wifi/BT 5.0
> - USB 2.0 host port (A)
> - USB 2.0 micro usb, OTG
> - USB 3.0 Host + 4 port USB hub (GL3510)
> - Gigabit Ethernet (Realtek RTL8211E phy)
> - HDMI 2.0 port
> - soldered eMMC (optional)
> - 3x LED (one is on the bottom)
> - microphone
> - audio jack
> - PCIe
>
> Add basic support for the board.
>
> Signed-off-by: Ondrej Jirman <megous@megous.com>
> ---
>  arch/arm64/boot/dts/allwinner/Makefile        |   1 +
>  .../dts/allwinner/sun50i-h6-orangepi-3.dts    | 212 ++++++++++++++++++
>  2 files changed, 213 insertions(+)
>  create mode 100644 arch/arm64/boot/dts/allwinner/sun50i-h6-orangepi-3.dts
>
> diff --git a/arch/arm64/boot/dts/allwinner/Makefile b/arch/arm64/boot/dts/allwinner/Makefile
> index e4dce2f6fa3a..285a7cb5135b 100644
> --- a/arch/arm64/boot/dts/allwinner/Makefile
> +++ b/arch/arm64/boot/dts/allwinner/Makefile
> @@ -20,6 +20,7 @@ dtb-$(CONFIG_ARCH_SUNXI) += sun50i-h5-orangepi-pc2.dtb
>  dtb-$(CONFIG_ARCH_SUNXI) += sun50i-h5-orangepi-prime.dtb
>  dtb-$(CONFIG_ARCH_SUNXI) += sun50i-h5-orangepi-zero-plus.dtb
>  dtb-$(CONFIG_ARCH_SUNXI) += sun50i-h5-orangepi-zero-plus2.dtb
> +dtb-$(CONFIG_ARCH_SUNXI) += sun50i-h6-orangepi-3.dtb
>  dtb-$(CONFIG_ARCH_SUNXI) += sun50i-h6-orangepi-lite2.dtb
>  dtb-$(CONFIG_ARCH_SUNXI) += sun50i-h6-orangepi-one-plus.dtb
>  dtb-$(CONFIG_ARCH_SUNXI) += sun50i-h6-pine-h64.dtb
> diff --git a/arch/arm64/boot/dts/allwinner/sun50i-h6-orangepi-3.dts b/arch/arm64/boot/dts/allwinner/sun50i-h6-orangepi-3.dts
> new file mode 100644
> index 000000000000..7a2424fcaed7
> --- /dev/null
> +++ b/arch/arm64/boot/dts/allwinner/sun50i-h6-orangepi-3.dts
> @@ -0,0 +1,212 @@
> +// SPDX-License-Identifier: (GPL-2.0+ or MIT)
> +/*
> + * Copyright (C) 2019 Ondřej Jirman <megous@megous.com>
> + */
> +
> +/dts-v1/;
> +
> +#include "sun50i-h6.dtsi"
> +
> +#include <dt-bindings/gpio/gpio.h>
> +
> +/ {
> +	model = "OrangePi 3";
> +	compatible = "xunlong,orangepi-3", "allwinner,sun50i-h6";

As Clement pointed out, this should be documented in
Documentation/devicetree/bindings/arm/sunxi.yaml

It's part of sunxi/for-next only at this point, and it will go through
a different branch than the H6 DTS, so it would be great to have it in
a separate patch.

> +	aliases {
> +		serial0 = &uart0;
> +	};
> +
> +	chosen {
> +		stdout-path = "serial0:115200n8";
> +	};
> +
> +	leds {
> +		compatible = "gpio-leds";
> +
> +		power {
> +			label = "orangepi:red:power";
> +			gpios = <&r_pio 0 4 GPIO_ACTIVE_HIGH>; /* PL4 */
> +			default-state = "on";
> +		};
> +
> +		status {
> +			label = "orangepi:green:status";
> +			gpios = <&r_pio 0 7 GPIO_ACTIVE_HIGH>; /* PL7 */
> +		};
> +	};
> +
> +	reg_vcc5v: vcc5v {
> +		/* board wide 5V supply directly from the DC jack */
> +		compatible = "regulator-fixed";
> +		regulator-name = "vcc-5v";
> +		regulator-min-microvolt = <5000000>;
> +		regulator-max-microvolt = <5000000>;
> +		regulator-always-on;
> +	};
> +};
> +
> +&cpu0 {
> +	cpu-supply = <&reg_dcdca>;
> +};
> +
> +&ehci0 {
> +	status = "okay";
> +};
> +
> +&ehci3 {
> +	status = "okay";
> +};
> +
> +&mmc0 {
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&mmc0_pins>;
> +	vmmc-supply = <&reg_cldo1>;
> +	cd-gpios = <&pio 5 6 GPIO_ACTIVE_LOW>; /* PF6 */
> +	bus-width = <4>;
> +	status = "okay";
> +};
> +
> +&ohci0 {
> +	status = "okay";
> +};
> +
> +&ohci3 {
> +	status = "okay";
> +};
> +
> +&pio {
> +	vcc-pc-supply = <&reg_bldo2>;
> +	vcc-pd-supply = <&reg_cldo1>;
> +};
> +
> +&r_i2c {
> +	status = "okay";
> +
> +	axp805: pmic@36 {
> +		compatible = "x-powers,axp805", "x-powers,axp806";
> +		reg = <0x36>;
> +		interrupt-parent = <&r_intc>;
> +		interrupts = <0 IRQ_TYPE_LEVEL_LOW>;
> +		interrupt-controller;
> +		#interrupt-cells = <1>;
> +		x-powers,self-working-mode;
> +		vina-supply = <&reg_vcc5v>;
> +		vinb-supply = <&reg_vcc5v>;
> +		vinc-supply = <&reg_vcc5v>;
> +		vind-supply = <&reg_vcc5v>;
> +		vine-supply = <&reg_vcc5v>;
> +		aldoin-supply = <&reg_vcc5v>;
> +		bldoin-supply = <&reg_vcc5v>;
> +		cldoin-supply = <&reg_vcc5v>;
> +
> +		regulators {
> +			reg_aldo1: aldo1 {
> +				regulator-always-on;
> +				regulator-min-microvolt = <3300000>;
> +				regulator-max-microvolt = <3300000>;
> +				regulator-name = "vcc-pl-led-ir";
> +			};
> +
> +			reg_aldo2: aldo2 {
> +				regulator-min-microvolt = <3300000>;
> +				regulator-max-microvolt = <3300000>;
> +				regulator-name = "vcc33-audio-tv-ephy-mac";
> +			};
> +
> +			/* ALDO3 is shorted to CLDO1 */
> +			reg_aldo3: aldo3 {
> +				regulator-always-on;
> +				regulator-min-microvolt = <3300000>;
> +				regulator-max-microvolt = <3300000>;
> +				regulator-name = "vcc33-io-pd-emmc-sd-usb-uart-1";
> +			};
> +
> +			reg_bldo1: bldo1 {
> +				regulator-always-on;
> +				regulator-min-microvolt = <1800000>;
> +				regulator-max-microvolt = <1800000>;
> +				regulator-name = "vcc18-dram-bias-pll";
> +			};
> +
> +			reg_bldo2: bldo2 {
> +				regulator-always-on;
> +				regulator-min-microvolt = <1800000>;
> +				regulator-max-microvolt = <1800000>;
> +				regulator-name = "vcc-efuse-pcie-hdmi-pc";
> +			};
> +
> +			bldo3 {
> +				/* unused */
> +			};
> +
> +			bldo4 {
> +				/* unused */
> +			};
> +
> +			reg_cldo1: cldo1 {
> +				regulator-always-on;
> +				regulator-min-microvolt = <3300000>;
> +				regulator-max-microvolt = <3300000>;
> +				regulator-name = "vcc33-io-pd-emmc-sd-usb-uart-2";
> +			};
> +
> +			cldo2 {
> +				/* unused */
> +			};
> +
> +			cldo3 {
> +				/* unused */
> +			};
> +
> +			reg_dcdca: dcdca {
> +				regulator-always-on;
> +				regulator-min-microvolt = <800000>;
> +				regulator-max-microvolt = <1160000>;
> +				regulator-name = "vdd-cpu";
> +			};
> +
> +			reg_dcdcc: dcdcc {
> +				regulator-min-microvolt = <810000>;
> +				regulator-max-microvolt = <1080000>;
> +				regulator-name = "vdd-gpu";
> +			};
> +
> +			reg_dcdcd: dcdcd {
> +				regulator-always-on;
> +				regulator-min-microvolt = <960000>;
> +				regulator-max-microvolt = <960000>;
> +				regulator-name = "vdd-sys";
> +			};
> +
> +			reg_dcdce: dcdce {
> +				regulator-always-on;
> +				regulator-min-microvolt = <1200000>;
> +				regulator-max-microvolt = <1200000>;
> +				regulator-name = "vcc-dram";
> +			};
> +
> +			sw {
> +				/* unused */
> +			};
> +		};
> +	};
> +};
> +
> +&uart0 {
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&uart0_ph_pins>;
> +	status = "okay";
> +};
> +
> +&usb2otg {
> +	dr_mode = "host";
> +	status = "okay";
> +};
> +
> +&usb2phy {
> +	usb0_id_det-gpios = <&pio 2 15 GPIO_ACTIVE_HIGH>; /* PC15 */
> +	usb0_vbus-supply = <&reg_vcc5v>;
> +	usb3_vbus-supply = <&reg_vcc5v>;
> +	status = "okay";

If we have an ID pin, then why is the OTG controller set to host?

Maxime

--
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
Maxime Ripard April 8, 2019, 8:47 a.m. UTC | #18
On Mon, Apr 08, 2019 at 03:28:24PM +0800, Chen-Yu Tsai wrote:
> On Mon, Apr 8, 2019 at 3:23 PM Maxime Ripard <maxime.ripard@bootlin.com> wrote:
> >
> > On Sat, Apr 06, 2019 at 01:45:04AM +0200, megous@megous.com wrote:
> > > From: Ondrej Jirman <megous@megous.com>
> > >
> > > Orange Pi 3 board requires enabling DDC I2C bus via some GPIO connected
> > > transistors, before it can be used. Model this as a power supply for DDC
> > > (via regulator framework).
> > >
> > > Signed-off-by: Ondrej Jirman <megous@megous.com>
> >
> > The DDC bus itself is usually attached to the HDMI connector, so it
> > would make sense to make the supply also a property of the connector.
>
> I believe these are separate things. What this patch covers is power for
> a voltage shifter between the SoC and HDMI DDC pins. The HDMI connector's
> 5V supply to power the remote DDC chip is something else. And on the
> Orange Pi 3 they are indeed separate supplies.

Then maybe the endpoint link between the two would be the best place
to put this?

Maxime

--
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
Ondřej Jirman April 8, 2019, 12:17 p.m. UTC | #19
On Mon, Apr 08, 2019 at 10:47:14AM +0200, Maxime Ripard wrote:
> On Mon, Apr 08, 2019 at 03:28:24PM +0800, Chen-Yu Tsai wrote:
> > On Mon, Apr 8, 2019 at 3:23 PM Maxime Ripard <maxime.ripard@bootlin.com> wrote:
> > >
> > > On Sat, Apr 06, 2019 at 01:45:04AM +0200, megous@megous.com wrote:
> > > > From: Ondrej Jirman <megous@megous.com>
> > > >
> > > > Orange Pi 3 board requires enabling DDC I2C bus via some GPIO connected
> > > > transistors, before it can be used. Model this as a power supply for DDC
> > > > (via regulator framework).
> > > >
> > > > Signed-off-by: Ondrej Jirman <megous@megous.com>
> > >
> > > The DDC bus itself is usually attached to the HDMI connector, so it
> > > would make sense to make the supply also a property of the connector.
> >
> > I believe these are separate things. What this patch covers is power for
> > a voltage shifter between the SoC and HDMI DDC pins. The HDMI connector's
> > 5V supply to power the remote DDC chip is something else. And on the
> > Orange Pi 3 they are indeed separate supplies.
> 
> Then maybe the endpoint link between the two would be the best place
> to put this?

Interestingly &hdmi node configures the DDC bus pins via pinctrl on the SoC
side, so I put this there too, because it's related to those pins. I'm not sure
if that changes anything in the discussion.

regards,
   o.

> Maxime
> 
> --
> Maxime Ripard, Bootlin
> Embedded Linux and Kernel engineering
> https://bootlin.com
Ondřej Jirman April 8, 2019, 12:46 p.m. UTC | #20
Hi Jagan,

On Mon, Apr 08, 2019 at 11:31:22AM +0530, Jagan Teki wrote:
> On Sun, Apr 7, 2019 at 8:02 PM 'Ondřej Jirman' via linux-sunxi
> <linux-sunxi@googlegroups.com> wrote:
> >
> > On Sun, Apr 07, 2019 at 03:36:21PM +0200, Clément Péron wrote:
> > > Hi,
> > >
> > > On Sat, 6 Apr 2019 at 01:45, megous via linux-sunxi
> > > <linux-sunxi@googlegroups.com> wrote:
> > > >
> > > > From: Ondrej Jirman <megous@megous.com>
> > > >
> > > > This series implements support for Xunlong Orange Pi 3 board.
> > >
> > > OrangePi 3 Lite2 and One Plus boards support has already been merged.
> > > The support is not complete but you should rebase your patches on top
> > > of sunxi/for-next
> >
> > Hi,
> >
> > OrangePi 3 is somewhat different from these two boards (mostly it has a differnt
> > power tree). It doesn't use the AXP regulators that are defined in the
> > sun50i-h6-orangepi.dtsi in the same way.
> >
> > For example:
> >
> > - bldo3 (is turned always on in sun50i-h6-orangepi.dtsi but unused for opi3)
> > - cldo2 and cldo3 are unused on opi3 and have nothing to do with WiFi
> > - aldo3 is not for dram
> > - bldo1 on the other hand is for dram on opi3
> > - some other regulators are used for different/more functions and thus
> >   named differntly
> > - USB id-det pin is differnt
> > - ...
> 
> Based on my communication with OrangePI, OPI-3 has PCIE, 4 USB-3.0
> ports and AV are the key differences and rest seems to be similar. but
> if we have a diff or unused regulators may be we can't enable them in
> dtsi (I never looked that close as of now)

These regulators are not just disabled on Opi 3, they have different/no meaning.

Schematics allow for high amount of variability in the power tree (see all the
NC (not connected) / 0R resistors) in the schematic around AXP805. Every board
based on this Xunlong design can be subtly different. 

I already suggested a maintainable solution, below. Where base dtsi has empty
config for regulators and every board based on that just defines it completely
for itself.

A few regulators (for CPU/GPU) will most probably have the same meaning on every
derived board, so these can probably be kept in dtsi without causing too much
annoyance.

It's unpleasant to have wrong regulator setup defined in an underlying dtsi, and
then trying to override it by removing/adding random properties in the board dts
for the new boards based on that, so that it fits.

The rest of the current HW descriptions in the sun50i-h6-orangepi.dtsi can be
shared (as of now).

thank you and regards,
  o.
Linus Walleij April 8, 2019, 12:53 p.m. UTC | #21
On Sat, Apr 6, 2019 at 1:45 AM <megous@megous.com> wrote:

> From: Ondrej Jirman <megous@megous.com>
>
> H6 has a different I/O voltage bias setting method than A80. Prepare
> existing code for using alternative bias voltage setting methods.
>
> Signed-off-by: Ondrej Jirman <megous@megous.com>
> ---
>  drivers/pinctrl/sunxi/pinctrl-sun9i-a80.c |  2 +-
>  drivers/pinctrl/sunxi/pinctrl-sunxi.c     | 38 ++++++++++++-----------
>  drivers/pinctrl/sunxi/pinctrl-sunxi.h     |  4 ++-

I need Maxime's ACK on these patches to merge.

Can the pinctrl parts be applied independently of the rest
of the changes when Maxime is happy with the patches?

Yours,
Linus Walleij
Ondřej Jirman April 8, 2019, 1:08 p.m. UTC | #22
Hi Linus,

On Mon, Apr 08, 2019 at 02:53:58PM +0200, Linus Walleij wrote:
> On Sat, Apr 6, 2019 at 1:45 AM <megous@megous.com> wrote:
> 
> > From: Ondrej Jirman <megous@megous.com>
> >
> > H6 has a different I/O voltage bias setting method than A80. Prepare
> > existing code for using alternative bias voltage setting methods.
> >
> > Signed-off-by: Ondrej Jirman <megous@megous.com>
> > ---
> >  drivers/pinctrl/sunxi/pinctrl-sun9i-a80.c |  2 +-
> >  drivers/pinctrl/sunxi/pinctrl-sunxi.c     | 38 ++++++++++++-----------
> >  drivers/pinctrl/sunxi/pinctrl-sunxi.h     |  4 ++-
> 
> I need Maxime's ACK on these patches to merge.
> 
> Can the pinctrl parts be applied independently of the rest
> of the changes when Maxime is happy with the patches?

Yes, the two pinctrl patches are independent of the rest of the series.

thank you,
	Ondrej

> Yours,
> Linus Walleij
Ondřej Jirman April 8, 2019, 10:26 p.m. UTC | #23
On Mon, Apr 08, 2019 at 11:41:38AM +0530, Jagan Teki wrote:
> On Sat, Apr 6, 2019 at 5:15 AM <megous@megous.com> wrote:
> >
> > From: Ondrej Jirman <megous@megous.com>
> >
> > Orange Pi 3 has two regulators that power the Realtek RTL8211E.
> > According to the phy datasheet, both regulators need to be enabled
> > at the same time, but we can only specify a single phy-supply in
> > the DT.
> >
> > This can be achieved by making one regulator depedning on the
> > other via vin-supply. While it's not a technically correct
> > description of the hardware, it achieves the purpose.
> >
> > All values of RX/TX delay were tested exhaustively and a middle
> > one of the working values was chosen.
> >
> > Signed-off-by: Ondrej Jirman <megous@megous.com>
> > ---
> >  .../dts/allwinner/sun50i-h6-orangepi-3.dts    | 44 +++++++++++++++++++
> >  1 file changed, 44 insertions(+)
> 
> This was in ML[1], I thought this change would already merged. I
> remember Chen-Yu applied and revert due to emac node not mainlined at
> that time.
> 
> [1] https://patchwork.kernel.org/patch/10558281/

The patch was not merged at the time, and bitrotted a bit. Armbian folks
were applying the bitortted patch out-of-the mainline and were experiencing
NPEs. I fixed the patch, and it is also part of this series. It's patch 5.

	o.
Ondřej Jirman April 8, 2019, 10:41 p.m. UTC | #24
On Mon, Apr 08, 2019 at 09:43:27AM +0200, Maxime Ripard wrote:
> On Sat, Apr 06, 2019 at 01:45:10AM +0200, megous@megous.com wrote:
> > From: Ondrej Jirman <megous@megous.com>
> >
> > ---
> >  arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi | 9 +++++++++
> >  1 file changed, 9 insertions(+)
> >
> > diff --git a/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi
> > index 91fecab58836..dccad79da90c 100644
> > --- a/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi
> > +++ b/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi
> > @@ -238,6 +238,15 @@
> >  				bias-pull-up;
> >  			};
> >
> > +
> 
> Extra line
> 
> > +			mmc1_pins: mmc1-pins {
> > +				pins = "PG0", "PG1", "PG2", "PG3",
> > +				       "PG4", "PG5";
> > +				function = "mmc1";
> > +				drive-strength = <30>;
> > +				bias-pull-up;
> > +			};
> > +
> 
> Is that the only muxing option?

I don't think so. I believe someone can use a 1-bit interface (bus-width = <1>),
and then some data pins will be free. This pinconfig is for 4-bit bus width
setup.

Though other SoCs (ex. H3, A83T) don't consider this possibility and make the
4-bit config the default pinctrl for mmc1. To add to the confusion, on these
SoCs 4-bit pinconf is the default, but 1bit bus-width is the (implicit) default.
This led to some confusion in the past.

So we can either:
- keep consistency with what is done elsewhere, and make this default, despite
  not being the only option,
- or perhaps I can rename this to mmc1_bus_width4_pins, or somesuch, to make it
  more explicit, and keep it non-default.

What do you think is better?

thank you and regards,
	o.

> If so, then it should be assigned by default to mmc1
> 
> Thanks!
> Maxime
> 
> --
> Maxime Ripard, Bootlin
> Embedded Linux and Kernel engineering
> https://bootlin.com
Ondřej Jirman April 8, 2019, 10:58 p.m. UTC | #25
On Mon, Apr 08, 2019 at 09:46:28AM +0200, Maxime Ripard wrote:
> On Sat, Apr 06, 2019 at 01:45:03AM +0200, megous@megous.com wrote:
> > From: Ondrej Jirman <megous@megous.com>
> >
> > Orange Pi 3 is a H6 based SBC made by Xulong, released in
> > January 2019. It has the following features:
> >
> > - Allwinner H6 quad-core 64-bit ARM Cortex-A53
> > - GPU Mali-T720
> > - 1GB or 2GB LPDDR3 RAM
> > - AXP805 PMIC
> > - AP6256 Wifi/BT 5.0
> > - USB 2.0 host port (A)
> > - USB 2.0 micro usb, OTG
> > - USB 3.0 Host + 4 port USB hub (GL3510)
> > - Gigabit Ethernet (Realtek RTL8211E phy)
> > - HDMI 2.0 port
> > - soldered eMMC (optional)
> > - 3x LED (one is on the bottom)
> > - microphone
> > - audio jack
> > - PCIe
> >
> > Add basic support for the board.
> >
> > Signed-off-by: Ondrej Jirman <megous@megous.com>
> > ---
> >  arch/arm64/boot/dts/allwinner/Makefile        |   1 +
> >  .../dts/allwinner/sun50i-h6-orangepi-3.dts    | 212 ++++++++++++++++++
> >  2 files changed, 213 insertions(+)
> >  create mode 100644 arch/arm64/boot/dts/allwinner/sun50i-h6-orangepi-3.dts
> >
> > diff --git a/arch/arm64/boot/dts/allwinner/Makefile b/arch/arm64/boot/dts/allwinner/Makefile
> > index e4dce2f6fa3a..285a7cb5135b 100644
> > --- a/arch/arm64/boot/dts/allwinner/Makefile
> > +++ b/arch/arm64/boot/dts/allwinner/Makefile
> > @@ -20,6 +20,7 @@ dtb-$(CONFIG_ARCH_SUNXI) += sun50i-h5-orangepi-pc2.dtb
> >  dtb-$(CONFIG_ARCH_SUNXI) += sun50i-h5-orangepi-prime.dtb
> >  dtb-$(CONFIG_ARCH_SUNXI) += sun50i-h5-orangepi-zero-plus.dtb
> >  dtb-$(CONFIG_ARCH_SUNXI) += sun50i-h5-orangepi-zero-plus2.dtb
> > +dtb-$(CONFIG_ARCH_SUNXI) += sun50i-h6-orangepi-3.dtb
> >  dtb-$(CONFIG_ARCH_SUNXI) += sun50i-h6-orangepi-lite2.dtb
> >  dtb-$(CONFIG_ARCH_SUNXI) += sun50i-h6-orangepi-one-plus.dtb
> >  dtb-$(CONFIG_ARCH_SUNXI) += sun50i-h6-pine-h64.dtb
> > diff --git a/arch/arm64/boot/dts/allwinner/sun50i-h6-orangepi-3.dts b/arch/arm64/boot/dts/allwinner/sun50i-h6-orangepi-3.dts
> > new file mode 100644
> > index 000000000000..7a2424fcaed7
> > --- /dev/null
> > +++ b/arch/arm64/boot/dts/allwinner/sun50i-h6-orangepi-3.dts
> > @@ -0,0 +1,212 @@
> > +// SPDX-License-Identifier: (GPL-2.0+ or MIT)
> > +/*
> > + * Copyright (C) 2019 Ondřej Jirman <megous@megous.com>
> > + */
> > +
> > +/dts-v1/;
> > +
> > +#include "sun50i-h6.dtsi"
> > +
> > +#include <dt-bindings/gpio/gpio.h>
> > +
> > +/ {
> > +	model = "OrangePi 3";
> > +	compatible = "xunlong,orangepi-3", "allwinner,sun50i-h6";
> 
> As Clement pointed out, this should be documented in
> Documentation/devicetree/bindings/arm/sunxi.yaml
> 
> It's part of sunxi/for-next only at this point, and it will go through
> a different branch than the H6 DTS, so it would be great to have it in
> a separate patch.
> 
> > +	aliases {
> > +		serial0 = &uart0;
> > +	};
> > +
> > +	chosen {
> > +		stdout-path = "serial0:115200n8";
> > +	};
> > +
> > +	leds {
> > +		compatible = "gpio-leds";
> > +
> > +		power {
> > +			label = "orangepi:red:power";
> > +			gpios = <&r_pio 0 4 GPIO_ACTIVE_HIGH>; /* PL4 */
> > +			default-state = "on";
> > +		};
> > +
> > +		status {
> > +			label = "orangepi:green:status";
> > +			gpios = <&r_pio 0 7 GPIO_ACTIVE_HIGH>; /* PL7 */
> > +		};
> > +	};
> > +
> > +	reg_vcc5v: vcc5v {
> > +		/* board wide 5V supply directly from the DC jack */
> > +		compatible = "regulator-fixed";
> > +		regulator-name = "vcc-5v";
> > +		regulator-min-microvolt = <5000000>;
> > +		regulator-max-microvolt = <5000000>;
> > +		regulator-always-on;
> > +	};
> > +};
> > +
> > +&cpu0 {
> > +	cpu-supply = <&reg_dcdca>;
> > +};
> > +
> > +&ehci0 {
> > +	status = "okay";
> > +};
> > +
> > +&ehci3 {
> > +	status = "okay";
> > +};
> > +
> > +&mmc0 {
> > +	pinctrl-names = "default";
> > +	pinctrl-0 = <&mmc0_pins>;
> > +	vmmc-supply = <&reg_cldo1>;
> > +	cd-gpios = <&pio 5 6 GPIO_ACTIVE_LOW>; /* PF6 */
> > +	bus-width = <4>;
> > +	status = "okay";
> > +};
> > +
> > +&ohci0 {
> > +	status = "okay";
> > +};
> > +
> > +&ohci3 {
> > +	status = "okay";
> > +};
> > +
> > +&pio {
> > +	vcc-pc-supply = <&reg_bldo2>;
> > +	vcc-pd-supply = <&reg_cldo1>;
> > +};
> > +
> > +&r_i2c {
> > +	status = "okay";
> > +
> > +	axp805: pmic@36 {
> > +		compatible = "x-powers,axp805", "x-powers,axp806";
> > +		reg = <0x36>;
> > +		interrupt-parent = <&r_intc>;
> > +		interrupts = <0 IRQ_TYPE_LEVEL_LOW>;
> > +		interrupt-controller;
> > +		#interrupt-cells = <1>;
> > +		x-powers,self-working-mode;
> > +		vina-supply = <&reg_vcc5v>;
> > +		vinb-supply = <&reg_vcc5v>;
> > +		vinc-supply = <&reg_vcc5v>;
> > +		vind-supply = <&reg_vcc5v>;
> > +		vine-supply = <&reg_vcc5v>;
> > +		aldoin-supply = <&reg_vcc5v>;
> > +		bldoin-supply = <&reg_vcc5v>;
> > +		cldoin-supply = <&reg_vcc5v>;
> > +
> > +		regulators {
> > +			reg_aldo1: aldo1 {
> > +				regulator-always-on;
> > +				regulator-min-microvolt = <3300000>;
> > +				regulator-max-microvolt = <3300000>;
> > +				regulator-name = "vcc-pl-led-ir";
> > +			};
> > +
> > +			reg_aldo2: aldo2 {
> > +				regulator-min-microvolt = <3300000>;
> > +				regulator-max-microvolt = <3300000>;
> > +				regulator-name = "vcc33-audio-tv-ephy-mac";
> > +			};
> > +
> > +			/* ALDO3 is shorted to CLDO1 */
> > +			reg_aldo3: aldo3 {
> > +				regulator-always-on;
> > +				regulator-min-microvolt = <3300000>;
> > +				regulator-max-microvolt = <3300000>;
> > +				regulator-name = "vcc33-io-pd-emmc-sd-usb-uart-1";
> > +			};
> > +
> > +			reg_bldo1: bldo1 {
> > +				regulator-always-on;
> > +				regulator-min-microvolt = <1800000>;
> > +				regulator-max-microvolt = <1800000>;
> > +				regulator-name = "vcc18-dram-bias-pll";
> > +			};
> > +
> > +			reg_bldo2: bldo2 {
> > +				regulator-always-on;
> > +				regulator-min-microvolt = <1800000>;
> > +				regulator-max-microvolt = <1800000>;
> > +				regulator-name = "vcc-efuse-pcie-hdmi-pc";
> > +			};
> > +
> > +			bldo3 {
> > +				/* unused */
> > +			};
> > +
> > +			bldo4 {
> > +				/* unused */
> > +			};
> > +
> > +			reg_cldo1: cldo1 {
> > +				regulator-always-on;
> > +				regulator-min-microvolt = <3300000>;
> > +				regulator-max-microvolt = <3300000>;
> > +				regulator-name = "vcc33-io-pd-emmc-sd-usb-uart-2";
> > +			};
> > +
> > +			cldo2 {
> > +				/* unused */
> > +			};
> > +
> > +			cldo3 {
> > +				/* unused */
> > +			};
> > +
> > +			reg_dcdca: dcdca {
> > +				regulator-always-on;
> > +				regulator-min-microvolt = <800000>;
> > +				regulator-max-microvolt = <1160000>;
> > +				regulator-name = "vdd-cpu";
> > +			};
> > +
> > +			reg_dcdcc: dcdcc {
> > +				regulator-min-microvolt = <810000>;
> > +				regulator-max-microvolt = <1080000>;
> > +				regulator-name = "vdd-gpu";
> > +			};
> > +
> > +			reg_dcdcd: dcdcd {
> > +				regulator-always-on;
> > +				regulator-min-microvolt = <960000>;
> > +				regulator-max-microvolt = <960000>;
> > +				regulator-name = "vdd-sys";
> > +			};
> > +
> > +			reg_dcdce: dcdce {
> > +				regulator-always-on;
> > +				regulator-min-microvolt = <1200000>;
> > +				regulator-max-microvolt = <1200000>;
> > +				regulator-name = "vcc-dram";
> > +			};
> > +
> > +			sw {
> > +				/* unused */
> > +			};
> > +		};
> > +	};
> > +};
> > +
> > +&uart0 {
> > +	pinctrl-names = "default";
> > +	pinctrl-0 = <&uart0_ph_pins>;
> > +	status = "okay";
> > +};
> > +
> > +&usb2otg {
> > +	dr_mode = "host";
> > +	status = "okay";
> > +};
> > +
> > +&usb2phy {
> > +	usb0_id_det-gpios = <&pio 2 15 GPIO_ACTIVE_HIGH>; /* PC15 */
> > +	usb0_vbus-supply = <&reg_vcc5v>;
> > +	usb3_vbus-supply = <&reg_vcc5v>;
> > +	status = "okay";
> 
> If we have an ID pin, then why is the OTG controller set to host?

This board has fixed conenction between VBUS and DCIN, so if it is powered
from DCIN and someone will try to connect it to the PC as a peripheral,
they'll get PC's VBUS shorted to the power supply connected to DCIN.

Depending on voltage difference between DCIN and PC's VBUS, you can get
overcurrent condidion and PC's port shutdown.

The board is not entirely foolproof in this regard.

- It can be host powered when connected via microUSB
- It can be self-powered and host an device on microUSB
- It can be self-powered and serve as a peripheral if you modify
  a cable (cut VBUS) or the host is expecting this and has some
  VBUS detection logic (most hosts will not have this)

I just didn't want to encourage use as a peripheral, because it's not
very foolproof. But I guess, DTS file will not stop anyone anyway.

I'll change it to otg, and maybe leave a small note.

regards,
	o.

> Maxime
> 
> --
> Maxime Ripard, Bootlin
> Embedded Linux and Kernel engineering
> https://bootlin.com
Ondřej Jirman April 8, 2019, 11:22 p.m. UTC | #26
On Mon, Apr 08, 2019 at 09:40:42AM +0200, Maxime Ripard wrote:
> On Sat, Apr 06, 2019 at 01:45:09AM +0200, megous@megous.com wrote:
> > From: Ondrej Jirman <megous@megous.com>
> >
> > Orange Pi 3 has two regulators that power the Realtek RTL8211E.
> > According to the phy datasheet, both regulators need to be enabled
> > at the same time, but we can only specify a single phy-supply in
> > the DT.
> >
> > This can be achieved by making one regulator depedning on the
> > other via vin-supply. While it's not a technically correct
> > description of the hardware, it achieves the purpose.
> >
> > All values of RX/TX delay were tested exhaustively and a middle
> > one of the working values was chosen.
> >
> > Signed-off-by: Ondrej Jirman <megous@megous.com>
> > ---
> >  .../dts/allwinner/sun50i-h6-orangepi-3.dts    | 44 +++++++++++++++++++
> >  1 file changed, 44 insertions(+)
> >
> > diff --git a/arch/arm64/boot/dts/allwinner/sun50i-h6-orangepi-3.dts b/arch/arm64/boot/dts/allwinner/sun50i-h6-orangepi-3.dts
> > index 644946749088..5270142527f5 100644
> > --- a/arch/arm64/boot/dts/allwinner/sun50i-h6-orangepi-3.dts
> > +++ b/arch/arm64/boot/dts/allwinner/sun50i-h6-orangepi-3.dts
> > @@ -15,6 +15,7 @@
> >
> >  	aliases {
> >  		serial0 = &uart0;
> > +		ethernet0 = &emac;
> >  	};
> >
> >  	chosen {
> > @@ -64,6 +65,27 @@
> >  		regulator-max-microvolt = <5000000>;
> >  		regulator-always-on;
> >  	};
> > +
> > +	/*
> > +	 * The board uses 2.5V RGMII signalling. Power sequence
> > +	 * to enable the phy is to enable GMAC-2V5 and GMAC-3V3 (aldo2)
> > +	 * power rails at the same time and to wait 100ms.
> > +	 */
> > +	reg_gmac_2v5: gmac-2v5 {
> > +                compatible = "regulator-fixed";
> > +                regulator-name = "gmac-2v5";
> > +                regulator-min-microvolt = <2500000>;
> > +                regulator-max-microvolt = <2500000>;
> > +                startup-delay-us = <100000>;
> > +                enable-active-high;
> > +                gpio = <&pio 3 6 GPIO_ACTIVE_HIGH>; /* PD6 */
> 
> Is enable-active-high still needed? It's redundant with the
> GPIO_ACTIVE_HIGH flag.

Looking at the code, use/non-use of enable-active-high inhibits
flags specified in gpio property. So the GPIO_ACTIVE_HIGH flag
is ignored here (had GPIO_ACTIVE_LOW been used, the kernel would
ignore it too and print a warning).

So enable-active-high is still necessary here.

See comment in gpiolib-of.c where this is handled:

/*
 * The regulator GPIO handles are specified such that the
 * presence or absence of "enable-active-high" solely controls
 * the polarity of the GPIO line. Any phandle flags must
 * be actively ignored.
 */

regards,
	o.

> The indentation is wrong here as well.
> 
> Maxime
> 
> --
> Maxime Ripard, Bootlin
> Embedded Linux and Kernel engineering
> https://bootlin.com
Maxime Ripard April 9, 2019, 7:20 a.m. UTC | #27
Hi,

On Tue, Apr 09, 2019 at 12:41:05AM +0200, Ondřej Jirman wrote:
> On Mon, Apr 08, 2019 at 09:43:27AM +0200, Maxime Ripard wrote:
> > On Sat, Apr 06, 2019 at 01:45:10AM +0200, megous@megous.com wrote:
> > > From: Ondrej Jirman <megous@megous.com>
> > >
> > > ---
> > >  arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi | 9 +++++++++
> > >  1 file changed, 9 insertions(+)
> > >
> > > diff --git a/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi
> > > index 91fecab58836..dccad79da90c 100644
> > > --- a/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi
> > > +++ b/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi
> > > @@ -238,6 +238,15 @@
> > >  				bias-pull-up;
> > >  			};
> > >
> > > +
> >
> > Extra line
> >
> > > +			mmc1_pins: mmc1-pins {
> > > +				pins = "PG0", "PG1", "PG2", "PG3",
> > > +				       "PG4", "PG5";
> > > +				function = "mmc1";
> > > +				drive-strength = <30>;
> > > +				bias-pull-up;
> > > +			};
> > > +
> >
> > Is that the only muxing option?
>
> I don't think so. I believe someone can use a 1-bit interface (bus-width = <1>),
> and then some data pins will be free. This pinconfig is for 4-bit bus width
> setup.
>
> Though other SoCs (ex. H3, A83T) don't consider this possibility and make the
> 4-bit config the default pinctrl for mmc1. To add to the confusion, on these
> SoCs 4-bit pinconf is the default, but 1bit bus-width is the (implicit) default.
> This led to some confusion in the past.
>
> So we can either:
> - keep consistency with what is done elsewhere, and make this default, despite
>   not being the only option,

What is done elsewhere is that if it's the only option, just call it
$controller_pins and make that the default. If it isn't, then call it
$(controller)_$(bank)_pins, and put it at the board level.

If it's not the only muxing option, then your name should be called
mmc1-pg-pins

> - or perhaps I can rename this to mmc1_bus_width4_pins, or somesuch, to make it
>   more explicit, and keep it non-default.

We haven't encountered a case where the 1-bit bus was actually used,
so there's no need to take care of that.

Maxime

--
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
Maxime Ripard April 9, 2019, 7:22 a.m. UTC | #28
On Tue, Apr 09, 2019 at 12:58:51AM +0200, Ondřej Jirman wrote:
> > > +&usb2otg {
> > > +	dr_mode = "host";
> > > +	status = "okay";
> > > +};
> > > +
> > > +&usb2phy {
> > > +	usb0_id_det-gpios = <&pio 2 15 GPIO_ACTIVE_HIGH>; /* PC15 */
> > > +	usb0_vbus-supply = <&reg_vcc5v>;
> > > +	usb3_vbus-supply = <&reg_vcc5v>;
> > > +	status = "okay";
> >
> > If we have an ID pin, then why is the OTG controller set to host?
>
> This board has fixed conenction between VBUS and DCIN, so if it is powered
> from DCIN and someone will try to connect it to the PC as a peripheral,
> they'll get PC's VBUS shorted to the power supply connected to DCIN.
>
> Depending on voltage difference between DCIN and PC's VBUS, you can get
> overcurrent condidion and PC's port shutdown.
>
> The board is not entirely foolproof in this regard.
>
> - It can be host powered when connected via microUSB
> - It can be self-powered and host an device on microUSB
> - It can be self-powered and serve as a peripheral if you modify
>   a cable (cut VBUS) or the host is expecting this and has some
>   VBUS detection logic (most hosts will not have this)
>
> I just didn't want to encourage use as a peripheral, because it's not
> very foolproof. But I guess, DTS file will not stop anyone anyway.
>
> I'll change it to otg, and maybe leave a small note.

Your solution was great actually. I guess making a comment explaining
what you just did would be better though, so that no one is confused.

Maxime

--
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
Maxime Ripard April 9, 2019, 7:23 a.m. UTC | #29
On Tue, Apr 09, 2019 at 01:22:32AM +0200, Ondřej Jirman wrote:
> On Mon, Apr 08, 2019 at 09:40:42AM +0200, Maxime Ripard wrote:
> > On Sat, Apr 06, 2019 at 01:45:09AM +0200, megous@megous.com wrote:
> > > From: Ondrej Jirman <megous@megous.com>
> > >
> > > Orange Pi 3 has two regulators that power the Realtek RTL8211E.
> > > According to the phy datasheet, both regulators need to be enabled
> > > at the same time, but we can only specify a single phy-supply in
> > > the DT.
> > >
> > > This can be achieved by making one regulator depedning on the
> > > other via vin-supply. While it's not a technically correct
> > > description of the hardware, it achieves the purpose.
> > >
> > > All values of RX/TX delay were tested exhaustively and a middle
> > > one of the working values was chosen.
> > >
> > > Signed-off-by: Ondrej Jirman <megous@megous.com>
> > > ---
> > >  .../dts/allwinner/sun50i-h6-orangepi-3.dts    | 44 +++++++++++++++++++
> > >  1 file changed, 44 insertions(+)
> > >
> > > diff --git a/arch/arm64/boot/dts/allwinner/sun50i-h6-orangepi-3.dts b/arch/arm64/boot/dts/allwinner/sun50i-h6-orangepi-3.dts
> > > index 644946749088..5270142527f5 100644
> > > --- a/arch/arm64/boot/dts/allwinner/sun50i-h6-orangepi-3.dts
> > > +++ b/arch/arm64/boot/dts/allwinner/sun50i-h6-orangepi-3.dts
> > > @@ -15,6 +15,7 @@
> > >
> > >  	aliases {
> > >  		serial0 = &uart0;
> > > +		ethernet0 = &emac;
> > >  	};
> > >
> > >  	chosen {
> > > @@ -64,6 +65,27 @@
> > >  		regulator-max-microvolt = <5000000>;
> > >  		regulator-always-on;
> > >  	};
> > > +
> > > +	/*
> > > +	 * The board uses 2.5V RGMII signalling. Power sequence
> > > +	 * to enable the phy is to enable GMAC-2V5 and GMAC-3V3 (aldo2)
> > > +	 * power rails at the same time and to wait 100ms.
> > > +	 */
> > > +	reg_gmac_2v5: gmac-2v5 {
> > > +                compatible = "regulator-fixed";
> > > +                regulator-name = "gmac-2v5";
> > > +                regulator-min-microvolt = <2500000>;
> > > +                regulator-max-microvolt = <2500000>;
> > > +                startup-delay-us = <100000>;
> > > +                enable-active-high;
> > > +                gpio = <&pio 3 6 GPIO_ACTIVE_HIGH>; /* PD6 */
> >
> > Is enable-active-high still needed? It's redundant with the
> > GPIO_ACTIVE_HIGH flag.
>
> Looking at the code, use/non-use of enable-active-high inhibits
> flags specified in gpio property. So the GPIO_ACTIVE_HIGH flag
> is ignored here (had GPIO_ACTIVE_LOW been used, the kernel would
> ignore it too and print a warning).
>
> So enable-active-high is still necessary here.

Too bad :/

> See comment in gpiolib-of.c where this is handled:
>
> /*
>  * The regulator GPIO handles are specified such that the
>  * presence or absence of "enable-active-high" solely controls
>  * the polarity of the GPIO line. Any phandle flags must
>  * be actively ignored.
>  */

Thanks for digging this out

Maxime

--
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
Linus Walleij April 9, 2019, 7:35 a.m. UTC | #30
On Tue, Apr 9, 2019 at 1:22 AM Ondřej Jirman <megous@megous.com> wrote:

> > > +                enable-active-high;
> > > +                gpio = <&pio 3 6 GPIO_ACTIVE_HIGH>; /* PD6 */
> >
> > Is enable-active-high still needed? It's redundant with the
> > GPIO_ACTIVE_HIGH flag.
>
> Looking at the code, use/non-use of enable-active-high inhibits
> flags specified in gpio property. So the GPIO_ACTIVE_HIGH flag
> is ignored here (had GPIO_ACTIVE_LOW been used, the kernel would
> ignore it too and print a warning).
>
> So enable-active-high is still necessary here.
>
> See comment in gpiolib-of.c where this is handled:
>
> /*
>  * The regulator GPIO handles are specified such that the
>  * presence or absence of "enable-active-high" solely controls
>  * the polarity of the GPIO line. Any phandle flags must
>  * be actively ignored.
>  */

Yeah this caused me special headache in the current merge
window because of buggy code on my part.

This is an effect of this flag being defined for powerpc
ages before we properly implemented generic GPIO
bindings. We just have to respect it.

See:
https://marc.info/?l=linux-gpio&m=155417774822532&w=2

Yours,
Linus Walleij
Maxime Ripard April 9, 2019, 7:45 a.m. UTC | #31
On Mon, Apr 08, 2019 at 02:17:27PM +0200, Ondřej Jirman wrote:
> On Mon, Apr 08, 2019 at 10:47:14AM +0200, Maxime Ripard wrote:
> > On Mon, Apr 08, 2019 at 03:28:24PM +0800, Chen-Yu Tsai wrote:
> > > On Mon, Apr 8, 2019 at 3:23 PM Maxime Ripard <maxime.ripard@bootlin.com> wrote:
> > > >
> > > > On Sat, Apr 06, 2019 at 01:45:04AM +0200, megous@megous.com wrote:
> > > > > From: Ondrej Jirman <megous@megous.com>
> > > > >
> > > > > Orange Pi 3 board requires enabling DDC I2C bus via some GPIO connected
> > > > > transistors, before it can be used. Model this as a power supply for DDC
> > > > > (via regulator framework).
> > > > >
> > > > > Signed-off-by: Ondrej Jirman <megous@megous.com>
> > > >
> > > > The DDC bus itself is usually attached to the HDMI connector, so it
> > > > would make sense to make the supply also a property of the connector.
> > >
> > > I believe these are separate things. What this patch covers is power for
> > > a voltage shifter between the SoC and HDMI DDC pins. The HDMI connector's
> > > 5V supply to power the remote DDC chip is something else. And on the
> > > Orange Pi 3 they are indeed separate supplies.
> >
> > Then maybe the endpoint link between the two would be the best place
> > to put this?
>
> Interestingly &hdmi node configures the DDC bus pins via pinctrl on the SoC
> side, so I put this there too, because it's related to those pins. I'm not sure
> if that changes anything in the discussion.

It's kind of different though. The DDC controller is within the HDMI
controller, which is inside the SoC. Just like the pin muxer. As far
as the hardware goes, even on your board, you don't need that supply
so that the signal gets out of the SoC.

If the regulator is to power up some component on the path between the
SoC and the connector, then it should be attached there.

Maxime

--
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com