Re: [PATCH v5 1/4] gpio: swnode: Add ability to specify native chip selects for SPI

[Andy Shevchenko <andy.shevchenko@gmail.com>]

On Thu, Apr 11, 2024 at 7:44 PM Charles Keepax
<ckeepax@opensource.cirrus.com> wrote:
> On Thu, Apr 11, 2024 at 04:25:25PM +0300, Andy Shevchenko wrote:
> > On Thu, Apr 11, 2024 at 12:06 PM Charles Keepax
> > <ckeepax@opensource.cirrus.com> wrote:

...

> > > +config GPIO_SWNODE_UNDEFINED
> > > +       bool
> >
> > But why did you remove the help? Please, put it back.
>
> Sorry didn't realise you still wanted it will pop it back.

No, I don't want it to be user-selectable.

This is defined by a non-empty summary near to the type of the option
(bool, tristate). The help text is orthogonal to this.

...

> > > +       if (!strcmp(gdev_node->name, GPIOLIB_SWNODE_UNDEFINED_NAME))
> > > +               return ERR_PTR(-ENOENT);
> >
> > This is a dead code when the respective config option is not selected.
> > Or actually a potential flaw if somebody else names their swnode like
> > this.
>
> Can add a check for the config.

Maybe something like

    if (IS_ENABLED(...) &&
        !strcmp(...))

...

> > > +       ret = software_node_register(&swnode_gpio_undefined);
> > > +       if (ret < 0)
> > > +               pr_err("gpiolib: failed to register swnode: %d\n", ret);
> >
> > Instead of this prefix, define pr_fmt()
>
> Little iffy on this, there are other prints in gpiolib that do it
> this way as well, I guess I could add a patch to convert
> everything but its starting to get a little out of the thrust of
> what I am doing here.

That's why I'm talking only about this (gpiolib-swnode) module where
you can have it as

  "gpiolib: swnode: " fmt

or alike

-- 
With Best Regards,
Andy Shevchenko