[PATCH v2 5/9] misc: qcom_geni: Add minicore support

[Simon Glass]

Hi Varadarajan,

On Sun, 12 Apr 2026 at 23:32, Varadarajan Narayanan
<varadarajan.narayanan at oss.qualcomm.com> wrote:
>
> Hi Simon,
>
> Thanks for the feedback, need one clarification...
>
> > > +     ...
> > > +     if (elf) {
> > > +             hdr = info;
> > > +             ...
> > > +     } else if (IS_ENABLED(CONFIG_QCOM_GENI_MINICORE)) {
> >
> > If elf is false and CONFIG_QCOM_GENI_MINICORE is disabled some
> > variables are used uninitialised. Please add an else clause that
> > returns an error.
>
> Ok.
>
> > > diff --git a/drivers/misc/qcom_geni.c b/drivers/misc/qcom_geni.c
> > > @@ -377,15 +413,22 @@ int qcom_geni_load_firmware(...)
> > > +     if (IS_ELF(*(Elf32_Ehdr *)fw)) {
> > > +             ...
> > > +             elf = 1;
> > > +             info = hdr;
> > > +     } else {
> > > +             elf = 0;
> > > +             info = fw;
> > > +     }
> >
> > Using 1/0 for a bool is a bit odd. Please use true/false.
>
> Ok.
>
> > > diff --git a/drivers/misc/qcom_geni.c b/drivers/misc/qcom_geni.c
> > > @@ -492,7 +540,11 @@ static int probe_children_load_firmware(...)
> > > +#if CONFIG_XPL_BUILD
> > > +int qcom_geni_fw_initialise(struct udevice *dev)
> > > +#else
> > > +int qcom_geni_fw_initialise(void)
> > > +#endif
> >
> > Having two different function signatures based on a preprocessor
> > conditional makes future maintenance harder. Consider always passing
> > dev and ignoring it when unused.
>
> The signature difference is because for one case it is
>
>         int (*probe)(struct udevice *dev);
>
> and
>
>         typedef int (*event_handler_simple_t)(void);
>
> for the other. Not sure how to reconcile between these two.
> Please advice.

One way is to use two separate stub functions (one for each signature)
which call a common function.

Regards,
Simon