Re: [PATCH] ARM: i.MX: Kamstrup mx7 concentrator board support

[Lars Pedersen <lapeddk@gmail.com>]

Hello Ahmad

On Mon, 12 Apr 2021 at 14:43, Ahmad Fatoum <a.fatoum@pengutronix.de> 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@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox