[En-Nut-Discussion] Problem with STM32 pin configuration

[Ole Reinhardt]

Hi Uwe,

Am 06.10.2015 11:46, schrieb Uwe Bonnes:
> Thanks for the hint. Argh, I stumbled on that problem before with the vendor
> headers :-(

Yes, I faced a similar problem some time ago in another context as well.
Very annoying, as the preprocessor does not complain.

> There are only the (5) places you pointed out where a compile time test for
> unused pins is done. With your solution, every of the about 200 pins needs a
> corresponding #define. Also pin name would vary when used for compile time
> vs. run-time usage. Let us consider if that is needed.
> 
> The test in stm32_emac.s can easily be rewritten as
>     if (Stm32GpioConfigSet(EMAC_PHY_CLOCK_MCO, GPIO_CFG_PERIPHAL |
>         GPIO_CFG_OUTPUT | EMAC_GPIO_SPEED, GPIO_AF_MCO) == 0) {
>         /* Output HSE clock (25MHz) on MCO pin (PA8) to clock the PHY */
>         RCC->CFGR = (RCC->CFGR & ~(RCC_CFGR_MCO1 | RCC_CFGR_MCO1PRE)) | RCC_CFGR_MCO1_1;
>     }
> without defines and with very low runtime impact.

Ok.

> The 4 tests for unconnected pins in stm32_spi.h are harder to work around,
> as long as the SPI driver uses per-device files.
> 
> In the long run, I tend to rewrite the spi-driver as one driver as
> stm32_i2cbus_v2.c already is. This will also require run-time decision on
> the pins used and remove the defines.

I think, this would be the most elegant solution, and as extra benefit
it saves code space, as stm32_spi.c ist not compiled in multiple times.

> On the short run, the setup seems broken anyhow:
> #if (SPI_CS0 != 0)
> #warning bla
> #define SPIBUS_SCK_PORT     (uint32_t)(stm32_port_nr2gpio[SPI_CS0  >> 8])
> #define SPIBUS_SCK_PIN      (SPI_CS0 & 0xf)
> ...
> This mixes SCK with CS0. Argh :-(

Oh :)

> Again, SPIBUS_CSx_INIT(x) can be used without compile-time defines with low
> run-time impact. For SPIBUS_CSx_SET and For SPIBUS_CSx_CLR I will probably
> introduce "void Stm32GpioSet(nutgpio_t gpio, int value)" as exported
> function. The function would return without setting anything if called with
> PIN_NONE. The run-time effect on  Stm32SpiChipSelect() should be barable.
> 
> I will try to fix and test.

Great, thanks.

> B.t.w. I in spibus.h, don't see a place to store private hardware data like
> in i2cbus.h:
>     /*! \brief Private data of the hardware specific implementation. */
>     void *bus_icb;

Indeed, the interface control block is missing there. I'd suggest to add
a bus_dcb _and_ bus_icb at the end of NUTSPIBUS, and add a node_icb at
the end of NUTSPINODE.

This way we should be fairly flexible in our implementation.

> Do I miss something? Otherwise, can we extend struct _NUTSPIBUS  for
>     void *bus_icb;
> at the end, without causing damage to others?

The above changes should not cause damage to others, but I'd like to
suggest to apply these changes to _all_ spi bus drivers, not only to our
stm32_spi driver and initialise them with NULL, where not needed.

Best regards and thanks for your help!

Ole

-- 
kernel concepts GmbH            Tel: +49-271-771091-14
Sieghuetter Hauptweg 48         Mob: +49-177-7420433
D-57072 Siegen
http://www.embedded-it.de
http://www.kernelconcepts.de