Aw: Re: [RFC v2 7/7] arm64: dts: mt7986: add Bananapi R3

[Frank Wunderlich]

Hi
> Gesendet: Freitag, 28. Oktober 2022 um 15:49 Uhr
> Von: "Daniel Golle" <daniel at makrotopia.org>

> On Fri, Oct 28, 2022 at 12:57:44PM +0200, Frank Wunderlich (linux) wrote:
> > Am 2022-10-28 11:19, schrieb AngeloGioacchino Del Regno:
> > > Il 26/10/22 11:36, Frank Wunderlich ha scritto:
> > > > From: Frank Wunderlich <frank-w at public-files.de>

> You could also use device tree overlays to select SDMMC or eMMC just
> like for the NOR vs. NAND choice on this board.

i recently found a way to compile dtbo via kernel compile-tools

https://github.com/frank-w/BPI-R2-4.14/commit/8a0d96d0932e71dd226b4cca641dcc097b23247c

is this the right way?

imho adding the mmc-node in base dts is better than getting device to boot only
with at least one overlay.

> > > > +		factory-key {
> > >
> > > I'd say that this is not "factory-key" but "reset-key"?
> >
> > okay i rename it.
> >
> > > > +			label = "reset";
> > > > +			linux,code = <KEY_RESTART>;
> > > > +			gpios = <&pio 9 GPIO_ACTIVE_LOW>;
>
> At least on my V1.0 board and reportedly also on V1.1 boards the RST
> button doesn't work. As soon as a NVME/M.2 module is inserted this
> also connects the GPIO just like if the button was pressed all the
> time. This issue has also been discussed in BananaPi forums.

maybe drop it for now till we can test it?

> > > > +	mmc0_pins_default: mmc0-pins {
> > > > +		mux {
> > > > +			function = "emmc";
> > > > +			groups = "emmc_51";
> > > > +		};
> > > > +		conf-cmd-dat {
> > > > +			pins = "EMMC_DATA_0", "EMMC_DATA_1", "EMMC_DATA_2",
> > > > +			       "EMMC_DATA_3", "EMMC_DATA_4", "EMMC_DATA_5",
> > > > +			       "EMMC_DATA_6", "EMMC_DATA_7", "EMMC_CMD";
> > > > +			input-enable;
> > > > +			drive-strength = <4>;
> > > > +			mediatek,pull-up-adv = <1>;	/* pull-up 10K */
> > >
> > > Can we please stop using these custom pull-{up,down}-adv properties?
> > > Check what was done on pinctrl-mt8192.c (and dt schema) for more
> > > information
> > > and examples.

as far as i see it defines array mt8192_pull_type with multiple MTK_PULL_PU_PD_TYPE / MTK_PULL_PUPD_R1R0_TYPE / MTK_PULL_PU_PD_RSEL_TYPE, but have not yet found out how it is working (array only put into mt8192_data.pull_type, no usage of this memeber, see no reference in mt8192.dtsi).

maybe with the

bias-pull-up = <MTK_PULL_SET_RSEL_011>;

in arch/arm64/boot/dts/mediatek/mt8192-asurada.dtsi is pointing to this array?

> > need to check these with MTK.
> >
> > > > +		};
> > > > +		conf-clk {

> > > > +&wifi {
> > > > +	status = "okay";
> > > > +	pinctrl-names = "default", "dbdc";
> > > > +	pinctrl-0 = <&wf_2g_5g_pins>, <&wf_led_pins>;
> > > > +	pinctrl-1 = <&wf_dbdc_pins>, <&wf_led_pins>;
> > > > +
> > > > +	mediatek,eeprom-data = <0x86790900 0xc4326 0x60000000 0x00 0x00
> > > > 0x00 0x00 0x00
> > >
> > > Ouch! This looks like firmware unrolled in a devicetree property - that
> > > can't
> > > be right.
> > >
> > > Please dump that in a binary file and load it as firmware from
> > > userspace.
> >
> > it uses the mt76 driver and here eeprom can only be loaded from
> > mtd-partition or from device tree. Previous attempts loading eeprom data
> > from userspace file (like it's done for "normal" firmware) were rejected.
>
> Note that strictly speaking this is not firmware but rather calibration
> data (ie. board-specific configuration, not code).
>
> In case you don't like the large amount of data in the DTS file, you
> can use the /incbin/ statement to include it from a file instead.

but this file needs to be inside the kernels tree to get the dts compiled.
As this is basicly a blob (and firmware is stored outside) this is tricky.

> As there is no physical EEPROM nor calibration data stored anywhere in
> he flash there is no easy way the driver could request a board-specific
> filename, unlike e.g. ath10k requesting BDF from userspace.
> In the past (e.g. out-of-tree patches for rt2x00 driver allowing it's
> use on Rt305x WiSoC) this lack of a board-specific filename has lead to
> people treating the file just like a generic firmware file: ignoring
> the board-specific nature of calibration data and just copying it from
> another board... As a work-around to prevent that, we could at least
> embed the filename in the dts or hack the driver to request a filename
> based on the top level 'compatible' string of the board's device tree.
>
> I've added Felix to Cc: as he suggested that solution back then.

maybe i should drop it for now to get basic support of the board without wifi till this is solved?

or adding wifi without the eeprom-data property and add firmware (file or directly like now) in my
own repo, but this needs to be done in every sourcebase as mainline is then non-functional.

regards Frank