| Message ID | 20220729104441.39177-8-angelogioacchino.delregno@collabora.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | MT6795 Devicetrees and Sony Xperia M5 | expand |
Il 29/07/22 14:00, Konrad Dybcio ha scritto: > > > On 29.07.2022 12:44, AngeloGioacchino Del Regno wrote: >> Add a basic support for the Sony Xperia M5 (codename "Holly") >> smartphone, powered by a MediaTek Helio X10 SoC. >> >> This achieves a console boot. >> >> Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com> Hello Konrad, First of all, I'm sorry for the very late reply. >> --- >> arch/arm64/boot/dts/mediatek/Makefile | 1 + >> .../dts/mediatek/mt6795-sony-xperia-m5.dts | 90 +++++++++++++++++++ >> 2 files changed, 91 insertions(+) >> create mode 100644 arch/arm64/boot/dts/mediatek/mt6795-sony-xperia-m5.dts >> >> diff --git a/arch/arm64/boot/dts/mediatek/Makefile b/arch/arm64/boot/dts/mediatek/Makefile >> index af362a085a02..72fd683c9264 100644 >> --- a/arch/arm64/boot/dts/mediatek/Makefile >> +++ b/arch/arm64/boot/dts/mediatek/Makefile >> @@ -3,6 +3,7 @@ dtb-$(CONFIG_ARCH_MEDIATEK) += mt2712-evb.dtb >> dtb-$(CONFIG_ARCH_MEDIATEK) += mt6755-evb.dtb >> dtb-$(CONFIG_ARCH_MEDIATEK) += mt6779-evb.dtb >> dtb-$(CONFIG_ARCH_MEDIATEK) += mt6795-evb.dtb >> +dtb-$(CONFIG_ARCH_MEDIATEK) += mt6795-sony-xperia-m5.dtb > -holly.dtb? > I prefer using the commercial name to identify the device. "Holly" is the smartphone project codename and that is mentioned almost nowhere: the aim here is to enhance readability as to make it immediately understandable that this devicetree is for the Xperia M5 device. >> dtb-$(CONFIG_ARCH_MEDIATEK) += mt6797-evb.dtb >> dtb-$(CONFIG_ARCH_MEDIATEK) += mt6797-x20-dev.dtb >> dtb-$(CONFIG_ARCH_MEDIATEK) += mt7622-rfb1.dtb >> diff --git a/arch/arm64/boot/dts/mediatek/mt6795-sony-xperia-m5.dts b/arch/arm64/boot/dts/mediatek/mt6795-sony-xperia-m5.dts >> new file mode 100644 >> index 000000000000..94d011c4126c >> --- /dev/null >> +++ b/arch/arm64/boot/dts/mediatek/mt6795-sony-xperia-m5.dts >> @@ -0,0 +1,90 @@ >> +// SPDX-License-Identifier: GPL-2.0-only >> +/* >> + * Copyright (c) 2022, Collabora Ltd >> + * Author: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com> >> + */ >> + >> +/dts-v1/; >> +#include "mt6795.dtsi" >> + >> +#include <dt-bindings/gpio/gpio.h> > Looks unused. > Right, I'll remove that in v2. >> + >> +/ { >> + model = "Sony Xperia M5"; >> + compatible = "sony,xperia-m5", "mediatek,mt6795"; > sony,holly? > I'm sorry, but I can't understand the sense of adding that compatible string to the mix. To the kernel, it doesn't mean anything - and we already have another string advertising the specific machine, which is "sony,xperia-m5". Of course, there is no Xperia M5 with a different SoC and, even if there was a xperia-m5 with a different SoC, we anyway have both a machine compatible and a SoC compatible in here, so that would still not pose any issue. >> + chassis-type = "handset"; >> + >> + aliases { >> + mmc0 = &mmc0; >> + mmc1 = &mmc1; >> + serial0 = &uart0; >> + serial1 = &uart1; >> + }; >> + >> + memory@40000000 { >> + device_type = "memory"; >> + reg = <0 0x40000000 0 0x1E800000>; > Lowercase hex in size. Also, doesn't the bootloader fill it in? > Updating the device to the latest software version will give you a bootloader that fills that in, but the first-ever software release contains one that will not do that in particular conditions (fastboot boot). >> + }; >> + >> + reserved_memory: reserved-memory { >> + #address-cells = <2>; >> + #size-cells = <2>; >> + ranges; >> + >> + /* 128 KiB reserved for ARM Trusted Firmware (BL31) */ > Is that true for all devices with this SoC, or..? If so, it may be worth > moving this into mt6795.dtsi. > >> + bl31_secmon_reserved: secmon@43000000 { > memory@, everywhere. Use labels to name the nodes. > I'm afraid that's not possible, as the bootloader is reading the devicetree and requires these nodes to follow this naming. >> + no-map; > reg goes first. Will fix in v2. Best regards, Angelo
On 27/10/2022 11:28, AngeloGioacchino Del Regno wrote: > Il 29/07/22 14:00, Konrad Dybcio ha scritto: >> >> >> On 29.07.2022 12:44, AngeloGioacchino Del Regno wrote: >>> Add a basic support for the Sony Xperia M5 (codename "Holly") >>> smartphone, powered by a MediaTek Helio X10 SoC. >>> >>> This achieves a console boot. >>> >>> Signed-off-by: AngeloGioacchino Del Regno >>> <angelogioacchino.delregno@collabora.com> > > Hello Konrad, > First of all, I'm sorry for the very late reply. > >>> --- >>> arch/arm64/boot/dts/mediatek/Makefile | 1 + >>> .../dts/mediatek/mt6795-sony-xperia-m5.dts | 90 >>> +++++++++++++++++++ >>> 2 files changed, 91 insertions(+) >>> create mode 100644 >>> arch/arm64/boot/dts/mediatek/mt6795-sony-xperia-m5.dts >>> >>> diff --git a/arch/arm64/boot/dts/mediatek/Makefile >>> b/arch/arm64/boot/dts/mediatek/Makefile >>> index af362a085a02..72fd683c9264 100644 >>> --- a/arch/arm64/boot/dts/mediatek/Makefile >>> +++ b/arch/arm64/boot/dts/mediatek/Makefile >>> @@ -3,6 +3,7 @@ dtb-$(CONFIG_ARCH_MEDIATEK) += mt2712-evb.dtb >>> dtb-$(CONFIG_ARCH_MEDIATEK) += mt6755-evb.dtb >>> dtb-$(CONFIG_ARCH_MEDIATEK) += mt6779-evb.dtb >>> dtb-$(CONFIG_ARCH_MEDIATEK) += mt6795-evb.dtb >>> +dtb-$(CONFIG_ARCH_MEDIATEK) += mt6795-sony-xperia-m5.dtb >> -holly.dtb? >> > > I prefer using the commercial name to identify the device. > "Holly" is the smartphone project codename and that is mentioned > almost nowhere: > the aim here is to enhance readability as to make it immediately > understandable > that this devicetree is for the Xperia M5 device. Ok, sounds good. > >>> dtb-$(CONFIG_ARCH_MEDIATEK) += mt6797-evb.dtb >>> dtb-$(CONFIG_ARCH_MEDIATEK) += mt6797-x20-dev.dtb >>> dtb-$(CONFIG_ARCH_MEDIATEK) += mt7622-rfb1.dtb >>> diff --git a/arch/arm64/boot/dts/mediatek/mt6795-sony-xperia-m5.dts >>> b/arch/arm64/boot/dts/mediatek/mt6795-sony-xperia-m5.dts >>> new file mode 100644 >>> index 000000000000..94d011c4126c >>> --- /dev/null >>> +++ b/arch/arm64/boot/dts/mediatek/mt6795-sony-xperia-m5.dts >>> @@ -0,0 +1,90 @@ >>> +// SPDX-License-Identifier: GPL-2.0-only >>> +/* >>> + * Copyright (c) 2022, Collabora Ltd >>> + * Author: AngeloGioacchino Del Regno >>> <angelogioacchino.delregno@collabora.com> >>> + */ >>> + >>> +/dts-v1/; >>> +#include "mt6795.dtsi" >>> + >>> +#include <dt-bindings/gpio/gpio.h> >> Looks unused. >> > > Right, I'll remove that in v2. > >>> + >>> +/ { >>> + model = "Sony Xperia M5"; >>> + compatible = "sony,xperia-m5", "mediatek,mt6795"; >> sony,holly? >> > > I'm sorry, but I can't understand the sense of adding that compatible > string to > the mix. To the kernel, it doesn't mean anything - and we already have > another > string advertising the specific machine, which is "sony,xperia-m5". I was suggesting replacing xperia-m5 with holly, but since we agreed on keeping m5 in the dtb name, I suppose it's fine for this one to stay too. > > Of course, there is no Xperia M5 with a different SoC and, even if > there was a > xperia-m5 with a different SoC, we anyway have both a machine > compatible and a > SoC compatible in here, so that would still not pose any issue. > >>> + chassis-type = "handset"; >>> + >>> + aliases { >>> + mmc0 = &mmc0; >>> + mmc1 = &mmc1; >>> + serial0 = &uart0; >>> + serial1 = &uart1; >>> + }; >>> + >>> + memory@40000000 { >>> + device_type = "memory"; >>> + reg = <0 0x40000000 0 0x1E800000>; >> Lowercase hex in size. Also, doesn't the bootloader fill it in? >> > > Updating the device to the latest software version will give you a > bootloader > that fills that in, but the first-ever software release contains one > that will > not do that in particular conditions (fastboot boot). Ugh. If only vendors tested their software before shipping it to users.. I think it's worth to adding a comment mentioning that, though. > >>> + }; >>> + >>> + reserved_memory: reserved-memory { >>> + #address-cells = <2>; >>> + #size-cells = <2>; >>> + ranges; >>> + >>> + /* 128 KiB reserved for ARM Trusted Firmware (BL31) */ >> Is that true for all devices with this SoC, or..? If so, it may be worth >> moving this into mt6795.dtsi. >> >>> + bl31_secmon_reserved: secmon@43000000 { >> memory@, everywhere. Use labels to name the nodes. >> > > I'm afraid that's not possible, as the bootloader is reading the > devicetree > and requires these nodes to follow this naming. Wow that's bad.. probably deserves a comment to prevent 'cleanups' breaking this. Konrad > >>> + no-map; >> reg goes first. > > Will fix in v2. > > Best regards, > Angelo > >
Il 27/10/22 11:40, Konrad Dybcio ha scritto: > > On 27/10/2022 11:28, AngeloGioacchino Del Regno wrote: >> Il 29/07/22 14:00, Konrad Dybcio ha scritto: >>> >>> >>> On 29.07.2022 12:44, AngeloGioacchino Del Regno wrote: >>>> Add a basic support for the Sony Xperia M5 (codename "Holly") >>>> smartphone, powered by a MediaTek Helio X10 SoC. >>>> >>>> This achieves a console boot. >>>> >>>> Signed-off-by: AngeloGioacchino Del Regno >>>> <angelogioacchino.delregno@collabora.com> >> >> Hello Konrad, >> First of all, I'm sorry for the very late reply. >> >>>> --- >>>> arch/arm64/boot/dts/mediatek/Makefile | 1 + >>>> .../dts/mediatek/mt6795-sony-xperia-m5.dts | 90 +++++++++++++++++++ >>>> 2 files changed, 91 insertions(+) >>>> create mode 100644 arch/arm64/boot/dts/mediatek/mt6795-sony-xperia-m5.dts >>>> >>>> diff --git a/arch/arm64/boot/dts/mediatek/Makefile >>>> b/arch/arm64/boot/dts/mediatek/Makefile >>>> index af362a085a02..72fd683c9264 100644 >>>> --- a/arch/arm64/boot/dts/mediatek/Makefile >>>> +++ b/arch/arm64/boot/dts/mediatek/Makefile >>>> @@ -3,6 +3,7 @@ dtb-$(CONFIG_ARCH_MEDIATEK) += mt2712-evb.dtb >>>> dtb-$(CONFIG_ARCH_MEDIATEK) += mt6755-evb.dtb >>>> dtb-$(CONFIG_ARCH_MEDIATEK) += mt6779-evb.dtb >>>> dtb-$(CONFIG_ARCH_MEDIATEK) += mt6795-evb.dtb >>>> +dtb-$(CONFIG_ARCH_MEDIATEK) += mt6795-sony-xperia-m5.dtb >>> -holly.dtb? >>> >> >> I prefer using the commercial name to identify the device. >> "Holly" is the smartphone project codename and that is mentioned almost nowhere: >> the aim here is to enhance readability as to make it immediately understandable >> that this devicetree is for the Xperia M5 device. > > Ok, sounds good. > > >> >>>> dtb-$(CONFIG_ARCH_MEDIATEK) += mt6797-evb.dtb >>>> dtb-$(CONFIG_ARCH_MEDIATEK) += mt6797-x20-dev.dtb >>>> dtb-$(CONFIG_ARCH_MEDIATEK) += mt7622-rfb1.dtb >>>> diff --git a/arch/arm64/boot/dts/mediatek/mt6795-sony-xperia-m5.dts >>>> b/arch/arm64/boot/dts/mediatek/mt6795-sony-xperia-m5.dts >>>> new file mode 100644 >>>> index 000000000000..94d011c4126c >>>> --- /dev/null >>>> +++ b/arch/arm64/boot/dts/mediatek/mt6795-sony-xperia-m5.dts >>>> @@ -0,0 +1,90 @@ >>>> +// SPDX-License-Identifier: GPL-2.0-only >>>> +/* >>>> + * Copyright (c) 2022, Collabora Ltd >>>> + * Author: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com> >>>> + */ >>>> + >>>> +/dts-v1/; >>>> +#include "mt6795.dtsi" >>>> + >>>> +#include <dt-bindings/gpio/gpio.h> >>> Looks unused. >>> >> >> Right, I'll remove that in v2. >> >>>> + >>>> +/ { >>>> + model = "Sony Xperia M5"; >>>> + compatible = "sony,xperia-m5", "mediatek,mt6795"; >>> sony,holly? >>> >> >> I'm sorry, but I can't understand the sense of adding that compatible string to >> the mix. To the kernel, it doesn't mean anything - and we already have another >> string advertising the specific machine, which is "sony,xperia-m5". > > I was suggesting replacing xperia-m5 with holly, but since we agreed on keeping > > m5 in the dtb name, I suppose it's fine for this one to stay too. > > >> >> Of course, there is no Xperia M5 with a different SoC and, even if there was a >> xperia-m5 with a different SoC, we anyway have both a machine compatible and a >> SoC compatible in here, so that would still not pose any issue. >> >>>> + chassis-type = "handset"; >>>> + >>>> + aliases { >>>> + mmc0 = &mmc0; >>>> + mmc1 = &mmc1; >>>> + serial0 = &uart0; >>>> + serial1 = &uart1; >>>> + }; >>>> + >>>> + memory@40000000 { >>>> + device_type = "memory"; >>>> + reg = <0 0x40000000 0 0x1E800000>; >>> Lowercase hex in size. Also, doesn't the bootloader fill it in? >>> >> >> Updating the device to the latest software version will give you a bootloader >> that fills that in, but the first-ever software release contains one that will >> not do that in particular conditions (fastboot boot). > > Ugh. If only vendors tested their software before shipping it to users.. > > I think it's worth to adding a comment mentioning that, though. > > >> >>>> + }; >>>> + >>>> + reserved_memory: reserved-memory { >>>> + #address-cells = <2>; >>>> + #size-cells = <2>; >>>> + ranges; >>>> + >>>> + /* 128 KiB reserved for ARM Trusted Firmware (BL31) */ >>> Is that true for all devices with this SoC, or..? If so, it may be worth >>> moving this into mt6795.dtsi. >>> Sorry again, I forgot to reply to this question, so addressing that now: no, that's not true for all devices with this SoC. I'm practically sure that all commercial devices that were shipped at that time require the same, but here upstream we also have a MT6795 dev board devicetree, which uses a much newer bootloader and possibly needs a different secmon carveout, if any at all. Hence, this one cannot be transferred to mt6795.dtsi.
diff --git a/arch/arm64/boot/dts/mediatek/Makefile b/arch/arm64/boot/dts/mediatek/Makefile index af362a085a02..72fd683c9264 100644 --- a/arch/arm64/boot/dts/mediatek/Makefile +++ b/arch/arm64/boot/dts/mediatek/Makefile @@ -3,6 +3,7 @@ dtb-$(CONFIG_ARCH_MEDIATEK) += mt2712-evb.dtb dtb-$(CONFIG_ARCH_MEDIATEK) += mt6755-evb.dtb dtb-$(CONFIG_ARCH_MEDIATEK) += mt6779-evb.dtb dtb-$(CONFIG_ARCH_MEDIATEK) += mt6795-evb.dtb +dtb-$(CONFIG_ARCH_MEDIATEK) += mt6795-sony-xperia-m5.dtb dtb-$(CONFIG_ARCH_MEDIATEK) += mt6797-evb.dtb dtb-$(CONFIG_ARCH_MEDIATEK) += mt6797-x20-dev.dtb dtb-$(CONFIG_ARCH_MEDIATEK) += mt7622-rfb1.dtb diff --git a/arch/arm64/boot/dts/mediatek/mt6795-sony-xperia-m5.dts b/arch/arm64/boot/dts/mediatek/mt6795-sony-xperia-m5.dts new file mode 100644 index 000000000000..94d011c4126c --- /dev/null +++ b/arch/arm64/boot/dts/mediatek/mt6795-sony-xperia-m5.dts @@ -0,0 +1,90 @@ +// SPDX-License-Identifier: GPL-2.0-only +/* + * Copyright (c) 2022, Collabora Ltd + * Author: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com> + */ + +/dts-v1/; +#include "mt6795.dtsi" + +#include <dt-bindings/gpio/gpio.h> + +/ { + model = "Sony Xperia M5"; + compatible = "sony,xperia-m5", "mediatek,mt6795"; + chassis-type = "handset"; + + aliases { + mmc0 = &mmc0; + mmc1 = &mmc1; + serial0 = &uart0; + serial1 = &uart1; + }; + + memory@40000000 { + device_type = "memory"; + reg = <0 0x40000000 0 0x1E800000>; + }; + + reserved_memory: reserved-memory { + #address-cells = <2>; + #size-cells = <2>; + ranges; + + /* 128 KiB reserved for ARM Trusted Firmware (BL31) */ + bl31_secmon_reserved: secmon@43000000 { + no-map; + reg = <0 0x43000000 0 0x30000>; + }; + + /* preloader and bootloader regions cannot be touched */ + preloader-region@44800000 { + no-map; + reg = <0 0x44800000 0 0x100000>; + }; + + bootloader-region@46000000 { + no-map; + reg = <0 0x46000000 0 0x400000>; + }; + }; +}; + +&pio { + uart0_pins: uart0-pins { + pins-rx { + pinmux = <PINMUX_GPIO113__FUNC_URXD0>; + bias-pull-up; + input-enable; + }; + pins-tx { + pinmux = <PINMUX_GPIO114__FUNC_UTXD0>; + output-high; + }; + }; + + uart2_pins: uart2-pins { + pins-rx { + pinmux = <PINMUX_GPIO31__FUNC_URXD2>; + bias-pull-up; + input-enable; + }; + pins-tx { + pinmux = <PINMUX_GPIO32__FUNC_UTXD2>; + }; + }; +}; + +&uart0 { + status = "okay"; + + pinctrl-names = "default"; + pinctrl-0 = <&uart0_pins>; +}; + +&uart2 { + status = "okay"; + + pinctrl-names = "default"; + pinctrl-0 = <&uart2_pins>; +};
Add a basic support for the Sony Xperia M5 (codename "Holly") smartphone, powered by a MediaTek Helio X10 SoC. This achieves a console boot. Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com> --- arch/arm64/boot/dts/mediatek/Makefile | 1 + .../dts/mediatek/mt6795-sony-xperia-m5.dts | 90 +++++++++++++++++++ 2 files changed, 91 insertions(+) create mode 100644 arch/arm64/boot/dts/mediatek/mt6795-sony-xperia-m5.dts