Hello again. Thanks for the quick response :) On Mon, 12 Apr 2021 at 13:24, Ahmad Fatoum <a.fatoum@xxxxxxxxxxxxxx> wrote: > > Hello Lars, > > On 12.04.21 12:45, Lars Pedersen wrote: > >>> +#define BOARD_RESTART_GPIO IMX_GPIO_NR(7, 12) > >>> +#define TPM_RESET_GPIO IMX_GPIO_NR(3, 8) > >>> + > >>> +static void kamstrup_mx7_tpm_reset(void) > >>> +{ > >>> + imx7_setup_pad(MX7D_PAD_LCD_DATA03__GPIO3_IO8); > >>> + > >>> + gpio_request(TPM_RESET_GPIO, "tpm-reset"); > >>> + gpio_direction_output(TPM_RESET_GPIO, 1); > >>> + mdelay(100); > >>> + gpio_set_value(TPM_RESET_GPIO, 0); > >>> + mdelay(100); > >>> + gpio_set_value(TPM_RESET_GPIO, 1); > >>> + gpio_free(TPM_RESET_GPIO); > >> > >> We are trying to cut down on code that doesn't use the driver model. > >> Couldn't this be represented as a gpio-hog in the device tree or > >> a reset line for the SPI device? > > > > I can't find anything in the DT binding for the SPI/TPM driver to use > > a reset line. > > Proper way would be for this to be added into Linux then, > but that's out of scope for the patch here. > > > Can a DT gpio-hog toggle the pin? Don't you need a > > driver for this? > > You can't pulse with a gpio-hog. I assumed the TPM is in reset by > default. > > > I see the following options: > > > > 1) gpio-hog with a label and use new gpiolib in board.c. > > 2) gpio-hog and control the pin in a boot script. > > If there's no straight-forward way to do it without board code, > it's ok the way it is with one change though: I missed it the first > time, but your board breaks multi-image support because you don't > check whether the initcall is indeed running on your board (See for > example imx_v7_defconfig, which builds over a hundred boards at once). > > Easiest way to get this right is to write a board driver. > See arch/arm/boards/lxa-mc1/board.c for an example. In arch/arm/boards/freescale-mx7-sabresd/board.c they do something like: if (!of_machine_is_compatible("fsl,imx7d-sdb")) return 0; Can't I just do the same? if (!of_machine_is_compatible("kam,imx7d-flex-concentrator-mfg")) return 0; The board driver in arch/arm/boards/lxa-mc1/board.c seems more complicated. > > >>> +} > >>> + > >>> +static void __noreturn kamstrup_mx7_board_restart_gpio(struct restart_handler *rst) > >>> +{ > >>> + imx7_setup_pad(MX7D_PAD_ENET1_TX_CLK__GPIO7_IO12); > >>> + > >>> + gpio_direction_output(BOARD_RESTART_GPIO, 0); > >>> + mdelay(1); > >>> + gpio_set_value(BOARD_RESTART_GPIO, 0); > >> > >> I just sent out a patch[1] with a driver implementing the "gpio-restart" device > >> tree binding. Could you test that one and use it here instead? > >> > > > > I have applied the patches on a master branch (Last patch failed to > > apply but was only watchdog) and the gpio-restart works perfectly. > > Thanks for testing! You can add your Tested-By on the patch in question > if you like. > > > > >>> + > >>> + hang(); > >>> +} > >>> + > >>> +static int kamstrup_mx7_concentrator_coredevices_init(void) > >>> +{ > >>> + kamstrup_mx7_tpm_reset(); > >>> + restart_handler_register_fn("board", kamstrup_mx7_board_restart_gpio); > >>> + barebox_set_model("Kamstrup OMNIA Concentrator"); > >> > >> The default model name is "Kamstrup OMNIA Flex Concentrator". > >> If that's too long, you could override /model in the barebox device tree. > >> With the changes suggested above, you could drop board.c then altogether. > >> > > > > I'll move this into the device tree or delete it entirely. > > Great. :-) > > > Cheers, > Ahmad > > -- > Pengutronix e.K. | | > Steuerwalder Str. 21 | http://www.pengutronix.de/ | > 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | > Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 | _______________________________________________ barebox mailing list barebox@xxxxxxxxxxxxxxxxxxx http://lists.infradead.org/mailman/listinfo/barebox