diff mbox series

[2/2] arm64: dts: meson: add g12a x96 max board

Message ID 20181219002620.12138-2-khilman@baylibre.com (mailing list archive)
State Not Applicable
Headers show
Series [1/2] dt-bindings: arm: amlogic: add amediatech x96-max bindings | expand

Commit Message

Kevin Hilman Dec. 19, 2018, 12:26 a.m. UTC
Add the G12a (S905X2) based X96 Max board[1].

There is no branding for the manufacturer anywhere on the product, so it
took some digging to find the manufacturer.  But since there's nothing
about the maker on the product I've left it out of the DT name because
1) nobody will no that name and 2) keeps the DT filename shorter.

[1] https://www.cnx-software.com/2018/09/25/x96-max-amlogic-s905x2-tv-box/

Cc: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
Signed-off-by: Kevin Hilman <khilman@baylibre.com>
---
Special thanks to Martin Blumenstingl for extra digging for the manufacturer

 .../boot/dts/amlogic/meson-g12a-x96-max.dts   | 28 +++++++++++++++++++
 1 file changed, 28 insertions(+)
 create mode 100644 arch/arm64/boot/dts/amlogic/meson-g12a-x96-max.dts

Comments

Peter Korsgaard Dec. 19, 2018, 4:13 p.m. UTC | #1
>>>>> "Kevin" == Kevin Hilman <khilman@baylibre.com> writes:

 > Add the G12a (S905X2) based X96 Max board[1].
 > There is no branding for the manufacturer anywhere on the product, so it
 > took some digging to find the manufacturer.  But since there's nothing
 > about the maker on the product I've left it out of the DT name because
 > 1) nobody will no that name and 2) keeps the DT filename shorter.

s/no/know/


 > [1] https://www.cnx-software.com/2018/09/25/x96-max-amlogic-s905x2-tv-box/

 > Cc: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
 > Signed-off-by: Kevin Hilman <khilman@baylibre.com>
 > ---
 > Special thanks to Martin Blumenstingl for extra digging for the manufacturer

 >  .../boot/dts/amlogic/meson-g12a-x96-max.dts   | 28 +++++++++++++++++++
 >  1 file changed, 28 insertions(+)
 >  create mode 100644 arch/arm64/boot/dts/amlogic/meson-g12a-x96-max.dts

 > diff --git a/arch/arm64/boot/dts/amlogic/meson-g12a-x96-max.dts
 > b/arch/arm64/boot/dts/amlogic/meson-g12a-x96-max.dts
 > new file mode 100644
 > index 000000000000..fe94765b671e
 > --- /dev/null
 > +++ b/arch/arm64/boot/dts/amlogic/meson-g12a-x96-max.dts
 > @@ -0,0 +1,28 @@
 > +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
 > +/*
 > + * Copyright (c) 2018 Amlogic, Inc. All rights reserved.

Amlogic? Shouldn't that be Baylibre?
Kevin Hilman Dec. 19, 2018, 11:31 p.m. UTC | #2
Peter Korsgaard <peter@korsgaard.com> writes:

>>>>>> "Kevin" == Kevin Hilman <khilman@baylibre.com> writes:
>
>  > Add the G12a (S905X2) based X96 Max board[1].
>  > There is no branding for the manufacturer anywhere on the product, so it
>  > took some digging to find the manufacturer.  But since there's nothing
>  > about the maker on the product I've left it out of the DT name because
>  > 1) nobody will no that name and 2) keeps the DT filename shorter.
>
> s/no/know/

Thanks.

>  > [1] https://www.cnx-software.com/2018/09/25/x96-max-amlogic-s905x2-tv-box/
>
>  > Cc: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
>  > Signed-off-by: Kevin Hilman <khilman@baylibre.com>
>  > ---
>  > Special thanks to Martin Blumenstingl for extra digging for the manufacturer
>
>  >  .../boot/dts/amlogic/meson-g12a-x96-max.dts   | 28 +++++++++++++++++++
>  >  1 file changed, 28 insertions(+)
>  >  create mode 100644 arch/arm64/boot/dts/amlogic/meson-g12a-x96-max.dts
>
>  > diff --git a/arch/arm64/boot/dts/amlogic/meson-g12a-x96-max.dts
>  > b/arch/arm64/boot/dts/amlogic/meson-g12a-x96-max.dts
>  > new file mode 100644
>  > index 000000000000..fe94765b671e
>  > --- /dev/null
>  > +++ b/arch/arm64/boot/dts/amlogic/meson-g12a-x96-max.dts
>  > @@ -0,0 +1,28 @@
>  > +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
>  > +/*
>  > + * Copyright (c) 2018 Amlogic, Inc. All rights reserved.
>
> Amlogic? Shouldn't that be Baylibre?

Yes.

Thanks for the review,

Kevin
Martin Blumenstingl Dec. 22, 2018, 4:57 p.m. UTC | #3
Hi Kevin,

On Wed, Dec 19, 2018 at 1:26 AM Kevin Hilman <khilman@baylibre.com> wrote:
>
> Add the G12a (S905X2) based X96 Max board[1].
>
> There is no branding for the manufacturer anywhere on the product, so it
> took some digging to find the manufacturer.  But since there's nothing
> about the maker on the product I've left it out of the DT name because
> 1) nobody will no that name and 2) keeps the DT filename shorter.
based on the cnx-soft article and my own board I would add the
following section:
-----
There are three variants of this device:
- 2GB LPDDR4, 16GB flash, 2.4G WiFi, 100M Ethernet
- 4GB LPDDR4, 32GB flash, dual band AC WiFi, Bluetooth 4.x, Gigabit Ethernet
- 4GB LPDDR4, 64GB flash, dual band  AC WiFi, Bluetooth 4.x, Gigabit Ethernet

Presumably the 4GB variants are identical except for the eMMC chip.

This patch is tested on the 4GB LPDDR4 32GB eMMC variant.
The WiFi / Bluetooth chip is an AP6356S. The Ethernet PHY is a Realtek RTL8211F.
-----

While writing this a question came up:
should we rename the board if there's a variant using the internal 100M PHY?
for example: "meson-g12a-x96-max-gbit"
this will allow us to add a "meson-g12a-x96-max-100m" (or any better
name) later on

> [1] https://www.cnx-software.com/2018/09/25/x96-max-amlogic-s905x2-tv-box/
>
> Cc: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
> Signed-off-by: Kevin Hilman <khilman@baylibre.com>
Tested-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
(with the Makefile adjustments, see below)

> ---
> Special thanks to Martin Blumenstingl for extra digging for the manufacturer
>
>  .../boot/dts/amlogic/meson-g12a-x96-max.dts   | 28 +++++++++++++++++++
>  1 file changed, 28 insertions(+)
it doesn't build the .dtb for me without adding the board to
arch/arm64/boot/dts/amlogic/Makefile

>  create mode 100644 arch/arm64/boot/dts/amlogic/meson-g12a-x96-max.dts
>
> diff --git a/arch/arm64/boot/dts/amlogic/meson-g12a-x96-max.dts b/arch/arm64/boot/dts/amlogic/meson-g12a-x96-max.dts
> new file mode 100644
> index 000000000000..fe94765b671e
> --- /dev/null
> +++ b/arch/arm64/boot/dts/amlogic/meson-g12a-x96-max.dts
> @@ -0,0 +1,28 @@
> +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
> +/*
> + * Copyright (c) 2018 Amlogic, Inc. All rights reserved.
> + */
> +
> +/dts-v1/;
> +
> +#include "meson-g12a.dtsi"
> +
> +/ {
> +       compatible = "amediatech,x96-max", "amlogic,u200", "amlogic,g12a";
did you leave the u200 compatible in there on purpose?

> +       model = "Shenzhen Amediatech Technology Co., Ltd X96 Max";
> +
> +       aliases {
> +               serial0 = &uart_AO;
> +       };
> +       chosen {
> +               stdout-path = "serial0:115200n8";
> +       };
> +       memory@0 {
> +               device_type = "memory";
> +               reg = <0x0 0x0 0x0 0x40000000>;
according to that cnx-soft report the smalles RAM size is 2GiB
however the bootloader will update it anyways so I believe this is fine


Regards
Martin
Martin Blumenstingl Jan. 8, 2019, 10:32 p.m. UTC | #4
Hi Kevin,

On Sat, Dec 22, 2018 at 5:57 PM Martin Blumenstingl
<martin.blumenstingl@googlemail.com> wrote:
>
> Hi Kevin,
>
> On Wed, Dec 19, 2018 at 1:26 AM Kevin Hilman <khilman@baylibre.com> wrote:
> >
> > Add the G12a (S905X2) based X96 Max board[1].
> >
> > There is no branding for the manufacturer anywhere on the product, so it
> > took some digging to find the manufacturer.  But since there's nothing
> > about the maker on the product I've left it out of the DT name because
> > 1) nobody will no that name and 2) keeps the DT filename shorter.
> based on the cnx-soft article and my own board I would add the
> following section:
> -----
> There are three variants of this device:
I had a chat with Chris Moore (who replied on the dt-bindings patch).

> - 2GB LPDDR4, 16GB flash, 2.4G WiFi, 100M Ethernet
> - 4GB LPDDR4, 32GB flash, dual band AC WiFi, Bluetooth 4.x, Gigabit Ethernet
I have the 4GB LPDDR4, 32G flash variant which uses:
RAM: SpecTek LPDDR4 (chip markings: PS082-062 BT / on the side it says R5JH)
eMMC: Micron (chip markings: 8FAI8 SJJML / on the side it says JXML ->
this might be "MTFC32GAKAECN-AIT")
WiFi: Ampak AP6356S

> - 4GB LPDDR4, 64GB flash, dual band  AC WiFi, Bluetooth 4.x, Gigabit Ethernet
Chris has the 4GB LPDDR4, 64GB flash variant.
RAM: same as on mine
eMMC: Samsung KLMCG4JENB-B041
WiFi: same as on mine


Regards
Martin
Martin Blumenstingl Jan. 9, 2019, 10:22 p.m. UTC | #5
Hi,

On Wed, Jan 9, 2019 at 1:11 AM Da Xue <da@lessconfused.com> wrote:
>
> I would be careful about adding TV boxes from unknown manufacturers in China. The manufacturer can and will change part numbers willy nilly and cause a ton of confusion. They are assembled with the cheapest part at the time so the DDR, WiFi module, eMMC, and even PCB will change without warning.
this is why I sent an update with the actual hardware which was
available for testing at the time this patch written / tested.

do you have any suggestion how to add support for this device while
being prepare for future changes?


Regards
Martin
Martin Blumenstingl Jan. 17, 2019, 7:55 p.m. UTC | #6
On Thu, Jan 10, 2019 at 7:54 PM Da Xue <da@lessconfused.com> wrote:
>
> Hi,
>
> To be on the safe side, I would add the PCB revision number and WiFi module name.
my PCB states: Q5X2 V3.0 18393

> Most of the WiFi modules are from Fn link so simply denoting the AP6356S may be sufficient to distinguish future PCB revisions and wireless modules.
our .dtbs mostly don't care about the WiFi chip (because it can be
auto-detected using the SDIO ID).
however, Bluetooth support on these modules is different because a
different WiFi / Bluetooth combo chip manufacturer also means
different dt-bindings

regarding the .dts name so far we have:
- meson-g12a-x96-max.dts
and your suggestions:
- meson-g12a-x96-max-ap6356s.dts
- meson-g12a-x96-max-3.0-ap6356s.dts


Regards
Martin
diff mbox series

Patch

diff --git a/arch/arm64/boot/dts/amlogic/meson-g12a-x96-max.dts b/arch/arm64/boot/dts/amlogic/meson-g12a-x96-max.dts
new file mode 100644
index 000000000000..fe94765b671e
--- /dev/null
+++ b/arch/arm64/boot/dts/amlogic/meson-g12a-x96-max.dts
@@ -0,0 +1,28 @@ 
+// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
+/*
+ * Copyright (c) 2018 Amlogic, Inc. All rights reserved.
+ */
+
+/dts-v1/;
+
+#include "meson-g12a.dtsi"
+
+/ {
+	compatible = "amediatech,x96-max", "amlogic,u200", "amlogic,g12a";
+	model = "Shenzhen Amediatech Technology Co., Ltd X96 Max";
+
+	aliases {
+		serial0 = &uart_AO;
+	};
+	chosen {
+		stdout-path = "serial0:115200n8";
+	};
+	memory@0 {
+		device_type = "memory";
+		reg = <0x0 0x0 0x0 0x40000000>;
+	};
+};
+
+&uart_AO {
+	status = "okay";
+};