[PATCH V3] ARM: dts: stm32: Add KS8851-16MLL ethernet on FMC2

[Marek Vasut]

On 4/1/20 3:30 PM, Patrick DELAUNAY wrote:
> Hi Marek,

Hi,

[...]

>> +&pinctrl {
>> +	/* These should bound to FMC2 bus driver, but we do not have one */
> 
> As temporarily solution (waiting final solution and real FMC2 bus driver) 
> can you define a NO_OP in board device associated to existing compatible
> = "st,stm32mp15-fmc2"

NO_OP ?

>> +	pinctrl-0 = <&fmc_pins_b>;
>> +	pinctrl-1 = <&fmc_sleep_pins_b>;
>> +	pinctrl-names = "default", "sleep";
>> +
>> +	fmc_pins_b: fmc-0 {
>> +		pins1 {
> [...]
>> +		};
>> +	};
>> +
>> +	fmc_sleep_pins_b: fmc-sleep-0 {
>> +		pins {
> [...]
>> +		};
>> +	};
>> +};
>> +
>>  &pmic {
>>  	u-boot,dm-pre-reloc;
>>  };
> 
> For example
> 
> &fmc {
> 	pinctrl-names = "default", "sleep";
> 	pinctrl-0 = <& fmc_pins_b>;
> 	pinctrl-1 = <& fmc_sleep_pins_b>;
> 	status = "okay";
> 	#address-cells = <1>;
> 	#size-cells = <0>;
> };
> 
> static const struct udevice_id stm32_fmc2_bus_ids[] = {
> 	{.compatible = "st,stm32mp15-fmc2},
> 	{ }
> };
> 
> U_BOOT_DRIVER(stm32_fmc2_bus) = {
> 	.name		= "stm32mp15-fmc2-ids",
> 	.id		= UCLASS_NOP,
> 	.of_match	= stm32_fmc2_bus_ids,
> 	.bind		= stm32_fmc2_bus,
> };

That looks like a hack, it would collide with the actual FMC2 driver and
it seems the FMC2 DT compatible string is not even stable yet (cfr the
Linux patches). So I am reluctant to do anything like depending on the
FMC DT bindings thus far.

>> diff --git a/board/dhelectronics/dh_stm32mp1/board.c
>> b/board/dhelectronics/dh_stm32mp1/board.c
>> index b663696983..be55242799 100644
>> --- a/board/dhelectronics/dh_stm32mp1/board.c
>> +++ b/board/dhelectronics/dh_stm32mp1/board.c
>> @@ -376,6 +376,32 @@ static void sysconf_init(void)  #endif  }
>>
>> +static void board_init_fmc2(void)
>> +{
> 
> Can you use device-tree information (to avoid hardcoded address STM32_FMC2_BASE).
> 
> For me, the address should be used only when the information are
> not available in device tree or before the device tree availability (in pre-reloc phasis)
> 
> ofnode = ofnode_by_compatible(ofnode_null(), "st,stm32mp15-fmc2");
> fmc2_addr = ofnode_get_addr(node);
> 
> or use NOP device as proposed previously.
> 
> PS: it is a preliminary step/temporarily solution, waiting FMC2 the bus driver availability.

Or, I can just use the address directly, which means no DT traversal, so
simpler code and faster boot time.

>> +#define STM32_FMC2_BCR1		0x0
>> +#define STM32_FMC2_BTR1		0x4
>> +#define STM32_FMC2_BWTR1	0x104
>> +#define STM32_FMC2_BCR(x)	((x) * 0x8 + STM32_FMC2_BCR1)
>> +#define STM32_FMC2_BTR(x)	((x) * 0x8 + STM32_FMC2_BTR1)
>> +#define STM32_FMC2_BWTR(x)	((x) * 0x8 + STM32_FMC2_BWTR1)
>> +
>> +#define RCC_MP_AHB6RSTCLRR	0x218
>> +#define RCC_MP_AHB6ENSETR	0x19c
>> +
>> +	/* Set up FMC2 bus for KS8851-16MLL and X11 SRAM */
>> +	writel(BIT(12), STM32_RCC_BASE + RCC_MP_AHB6RSTCLRR);
>> +	writel(BIT(12), STM32_RCC_BASE + RCC_MP_AHB6ENSETR);
> 
> Use clk and reset driver model and DT, for example:
> 
> struct clk clk;
> struct reset_ctl reset;
> 
> clk_get_by_index_nodev(ofnode, 0, &clk ;
> clk_get_by_index_nodev(ofnode, 0,  &reset);

This very much looks like I can just write the entire bus driver by now,
instead of just writing these few registers in a real simple way, until
the bus driver exists.

But since the bindings aren't stable, I am not very inclined to do that.

[...]