optional load modes for iCE40

[benediktibk]

This makes the two load modes for the iCE40 optional and represents therefore a workaround for #77983.

[benediktibk]

based upon #80846

[josuah]

I really like the direction it is taking, but wonder if it would be interesting to split this in 2 drivers in the same file, using two different compatibles:

#define ICE40_FIELDS_COMMON(inst) \
		.creset = GPIO_DT_SPEC_INST_GET(inst, creset_gpios),               \
		.cdone = GPIO_DT_SPEC_INST_GET(inst, cdone_gpios),                 \
		.mhz_delay_count = FPGA_ICE40_MHZ_DELAY_COUNT(inst),               \
		.config_delay_us = FPGA_ICE40_CONFIG_DELAY_US(inst),               \
		.creset_delay_us = FPGA_ICE40_CRESET_DELAY_US(inst),               \
		.leading_clocks = FPGA_ICE40_LEADING_CLOCKS(inst),                 \
		.trailing_clocks = FPGA_ICE40_TRAILING_CLOCKS(inst),               \


#define ICE40_DEFINE_SPI(inst) \
	static struct fpga_ice40_data fpga_ice40_data_##inst;                      \
                                                                                   \
	static const struct fpga_ice40_config fpga_ice40_config_##inst = {         \
		ICE40_FIELDS_COMMON(inst)                                          \
		.bus = SPI_DT_SPEC_INST_GET(inst, SPI_WORD_SET(8) | SPI_TRANSFER_MSB, 0),\
		.load = fpga_ice40_load_spi};                                      \
                                                                                   \
	DEVICE_DT_INST_DEFINE(inst, fpga_ice40_init, NULL, &fpga_ice40_data_##inst,\
			      &fpga_ice40_config_##inst, POST_KERNEL, CONFIG_FPGA_INIT_PRIORITY,\
			      &fpga_ice40_api);

#define ICE40_DEFINE_GPIO(inst) \
	FPGA_ICE40_PINCTRL_DEFINE(inst);                                           \
                                                                                   \
	static struct fpga_ice40_data fpga_ice40_data_##inst;                      \
                                                                                   \
	static const struct fpga_ice40_config fpga_ice40_config_##inst = {         \
		ICE40_FIELDS_COMMON(inst)                                          \
		.clk = GPIO_DT_SPEC_INST_GET_OR(inst, clk_gpios, {0}),             \
		.pico = GPIO_DT_SPEC_INST_GET_OR(inst, pico_gpios, {0}),           \
		.set = FPGA_ICE40_GPIO_PINS(inst, gpios_set_reg),                  \
		.clear = FPGA_ICE40_GPIO_PINS(inst, gpios_clear_reg),              \
		.load = fpga_ice40_load_gpio,                                      \
		FPGA_ICE40_PINCTRL_CONFIG(inst)};                                  \
                                                                                   \
	DEVICE_DT_INST_DEFINE(inst, fpga_ice40_init, NULL, &fpga_ice40_data_##inst,\
			      &fpga_ice40_config_##inst, POST_KERNEL, CONFIG_FPGA_INIT_PRIORITY,\
			      &fpga_ice40_api);


#define DT_DRV_COMPAT lattice_ice40_spi
DT_INST_FOREACH_STATUS_OKAY(ICE40_DEFINE)
#undef DT_DRV_COMPAT

#define DT_DRV_COMPAT lattice_ice40_gpio
DT_INST_FOREACH_STATUS_OKAY(ICE40_DEFINE)
#undef DT_DRV_COMPAT

That is a breaking change, though, and the change you introduced are not: +1 with/without splitting the bindings.

[benediktibk]

I really like the direction it is taking, but wonder if it would be interesting to split this in 2 drivers in the same file, using two different compatibles:

Interesting idea, and the breaking change should not be a big issue as the driver is still experimental. @cfriedt Preferences from your side?

[cfriedt]

Yeah, until we have better support for deinitializing SPI controllers, I guess anything is better than not having multi-instance.

Let's try two DT compats - why not?

[benediktibk]

Yeah, until we have better support for deinitializing SPI controllers, I guess anything is better than not having multi-instance.

Let's try two DT compats - why not?

Okay, I will do so. But I'm gonna wait until #81212 is merged to avoid a conflict.

[benediktibk]

I've separated the drivers into a SPI and GPIO bitbang implementation, with two different compatibles. I've kept the original binding as it is to avoid necessary changes downstream.

[benediktibk]

Added a bullet point in the release notes for the change in the binding.

[josuah]

Thank you very much for this effort. I could as well be doing it since I suggested the changes...

The situation seems very clear now. Alot of #ifdef and BUILD_ASSERT() removed.

[benediktibk]

@cfriedt Btw, I've removed the experimental flag from the SPI-based driver, as I think the experimental state at least for this driver should be gone with this state.

[benediktibk]

The compliance check error was interesting:

/bin/sh: 1: gitlint: not found

Did a rebase onto main to trigger the CI.