Hello Ahmad On Mon, 12 Apr 2021 at 14:43, Ahmad Fatoum <a.fatoum@xxxxxxxxxxxxxx> wrote: > > Hello Lars, > > On 12.04.21 14:09, Lars Pedersen wrote: > > Hello again. Thanks for the quick response :) > >> 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. > > The board driver support is recent and not many boards use it. > Doing of_machine_is_compatible check manually is error-prone (inverted logic, > sometimes it's forgotten, like in v1 here) and doesn't scale once you > have multiple compatibles you want to match against. Drivers can also > EPROBE_DEFER if they can't yet acquire a resource. > > For the simple case of toggling a reset, likely only difference now is that you > can verify your driver availability with drvinfo. I'd prefer new code uses board > from the get-go though if board code is really needed. > I got it working but we have decided to update the hardware to reset the TPM after your colleague mentioned how it should be done correctly :) Just to be clear. I can now remove board.c as you first mentioned since I don't need any special stuff. Also I need to update imx_v7_defconfig with this board with default value yes. > Cheers, > Ahmad > > > > >> > >>>>> +} > >>>>> + > >>>>> +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 | > > > > -- > 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