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