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.
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
