| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <cafdf662-6f15-8e16-7752-5740f28b6682@baylibre.com>
Date: Fri, 30 Apr 2021 10:16:48 +0200
From: Neil Armstrong <narmstrong@...libre.com>
To: Martin Blumenstingl <martin.blumenstingl@...glemail.com>
Cc: khilman@...libre.com, jbrunet@...libre.com,
linux-amlogic@...ts.infradead.org,
linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 3/3] arm64: dts: meson-sm1: add Banana PI BPI-M5 board dts
Hi Martin,
On 29/04/2021 23:30, Martin Blumenstingl wrote:
> Hi Neil,
>
> thanks for adding support for the BPI-M5!
> I had this on my TODO-list for a long time but didn't have enough time
> so far. so it's great to see this patch now :-)
Same, so it's nearly done now !
>
> On Thu, Apr 29, 2021 at 7:04 PM Neil Armstrong <narmstrong@...libre.com> wrote:
> [...]
>> + adc_keys {
>> + compatible = "adc-keys";
>> + io-channels = <&saradc 2>;
>> + io-channel-names = "buttons";
>> + keyup-threshold-microvolt = <1800000>;
>> +
>> + button-onoff {
> maybe just "key" (as you used below for SW1)?
Fixed
>
>> + label = "SW3";
>> + linux,code = <BTN_3>;
>> + press-threshold-microvolt = <1700000>;
>> + };
>> + };
>
>> + /* TOFIX: handle CVBS_DET on SARADC channel 0 */
> it's great to see that after many years at least some boards finally
> can detect whether the CVBS connector is plugged it
Yes, it's also available on Odroid-N2(+), so now no excuse for not supporting it !
>
>> + memory@0 {
>> + device_type = "memory";
>> + reg = <0x0 0x0 0x0 0x40000000>;
>> + };
> note to self: u-boot will update this from 1GiB to 4GiB
>
>> + flash_1v8: regulator-flash_1v8 {
>> + compatible = "regulator-fixed";
>> + regulator-name = "FLASH_1V8";
> BPI-M5-SCH-V10-Release.pdf schematics are calling this EMMC_1V8. I
> suggest to use that name so it's easier to compare this with the
> schematics in the future
Done
>
>> + regulator-min-microvolt = <1800000>;
>> + regulator-max-microvolt = <1800000>;
>> + vin-supply = <&vddao_3v3>;
>> + regulator-always-on;
>> + };
>> +
>> + dc_in: regulator-dc_in {
>> + compatible = "regulator-fixed";
>> + regulator-name = "5V";
> maybe use "DC_IN" here as well so the node-name matches what we see in
> userspace/kernel logs?
Done
>
>> + vddcpu: regulator-vddcpu {
>> + /*
>> + * SY8120B1ABC DC/DC Regulator.
>> + */
>> + compatible = "pwm-regulator";
>> +
>> + regulator-name = "VDDCPU";
>> + regulator-min-microvolt = <721000>;
>> + regulator-max-microvolt = <1022000>;
> the vendor .dts has:
> regulator-min-microvolt = <690000>;
> regulator-max-microvolt = <1050000>;
> which also matches meson-sm1-sei610.dts (which uses the same regulator IC)
Done, thanks
>
> [...]
>> +ðmac {
>> + pinctrl-0 = <ð_pins>, <ð_rgmii_pins>;
>> + pinctrl-names = "default";
>> + status = "okay";
>> + phy-mode = "rgmii";
>> + phy-handle = <&external_phy>;
>> + amlogic,tx-delay-ns = <2>;
> I haven't checked their u-boot code but some of the newer Amlogic BSPs
> seem to let the PHY add the TX delay (which is also what the netdev
> maintainers recommend)
> that can be achieved by setting:
> phy-mode = "rgmii-txid";
> and deleting the "amlogic,tx-delay-ns" property
Done, but what about when we use mainline U-boot here ?
>
>> +&gpio {
>> + gpio-line-names =
>> + /* GPIOZ */
>> + "", "", "", "", "", "", "", "",
> GPIOZ_0 to GPIOZ_15:
> - ETH_MDIO
> - ETH_MDC
> - ETH_RXCLK
> - ETH_RX_DV
> - ETH_RXD0
> - ETH_RXD1
> - ETH_RXD2
> - ETH_RXD3
> - ETH_TXCLK
> - ETH_TXEN
> - ETH_TXD0
> - ETH_TXD1
> - ETH_TXD2
> - ETH_TXD3
> - ETH_INTR
> - ETH_NRST
>
>> + /* GPIOH */
>> + "", "", "", "", "",
> GPIOH_0 to GPIOH_4:
> - HDMI_SDA
> - HDMI_SCL
> - HDMI_HPD
> - HDMI_CEC
> - VL-RST_N
>
>> + "CON1-P36", /* GPIOH_5 */
> GPIOH_6 to GPIOH_8:
> - VL-PWREN
> - WiFi_3V3_1V8
> - TFLASH_VDD_EN
>
>> + /* BOOT */
>> + "", "", "", "", "", "", "", "",
>> + "", "", "", "", "", "", "", "",
> BOOT_0 to BOOT_13:
> - eMMC_D0
> - eMMC_D1
> - eMMC_D2
> - eMMC_D3
> - eMMC_D4
> - eMMC_D5
> - eMMC_D6
> - eMMC_D7
> - eMMC_CLK
> - (BOOT_9 is unused)
> - eMMC_CMD
> - (BOOT_11 is unused)
> - eMMC_RST#
> - eMMC_DS
>
>> + /* GPIOC */
>> + "", "", "", "", "", "", "", "",
> GPIOC_0 to GPIOC_7:
> - SD_D0_B
> - SD_D1_B
> - SD_D2_B
> - SD_D3_B
> - SD_CLK_B
> - SD_CMD_B
> - CARD_EN_DET
> - (GPIOC_7 is unused)
>
>
>> +&gpio_ao {
>> + gpio-line-names =
>> + /* GPIOAO */
>> + "DEBUG TX", /* GPIOAO_0 */
>> + "DEBUG RX", /* GPIOAO_1 */
>> + "SYS_LED2", /* GPIOAO_2 */
>> + "UPDATE_KEY", /* GPIOAO_3 */
>> + "CON1-P40", /* GPIOAO_4 */
>> + "",
> GPIOAO_5 is IR_IN
>
>> + "TF_3V3N_1V8_EN", /* GPIOAO_6 */
>> + "CON1-P35", /* GPIOAO_7 */
>> + "CON1-P12", /* GPIOAO_8 */
>> + "CON1-P37", /* GPIOAO_9 */
>> + "CON1-P38", /* GPIOAO_10 */
>> + "SYS_LED", /* GPIOAO_11 */
>> + /* GPIOE */
>> + "", "", "";
> GPIOE_0 to GPIOOE_2:
> - VDDEE_PWM
> - VDDCPU_PWM
> - TF_PWR_EN
I was to lazy to do that, so thanks !
>
>> +&usb2_phy0 {
>> + phy-supply = <&dc_in>;
>> +};
>> +
>> +&usb2_phy1 {
>> + /* Enable the hub which is connected to this port */
>> + phy-supply = <&vl_pwr_en>;
>> +};
> I think technically we'd need to use AVDD18_USB here (that said, I
> chose the same approach as you are using here before..)
> USB bus devices (AFAIK) still need to be detected before any
> device-tree properties can be applied and there's (again AFAIK) still
> no pwrseq concept.
> so I guess for now this is the way to go as there's enough comments
> about it in your patch
yeah we'll see if there is proper pwrseq method to fix that (and the hub reset)
Thanks for the review,
Neil
>
>
> Best regards,
> Martin
>
Powered by blists - more mailing lists