Re: [PATCH v8 3/6] software node: implement reference properties

[Andy Shevchenko <andriy.shevchenko@linux.intel.com>]

On Mon, Nov 09, 2020 at 08:47:14PM +0100, Lukasz Stelmach wrote:
> It was <2020-11-09 pon 21:02>, when Andy Shevchenko wrote:
> > On Mon, Nov 09, 2020 at 07:18:37PM +0100, Lukasz Stelmach wrote:
> >> It was <2020-11-09 pon 19:24>, when Andy Shevchenko wrote:
> >> > On Mon, Nov 09, 2020 at 06:02:29PM +0100, Lukasz Stelmach wrote:
> >> >> It was <2019-11-07 czw 20:22>, when Dmitry Torokhov wrote:

...

> > Create GPIO lookup table.
> >
> >> I could use platform_data to pass structures from configfs but
> >> software nodes would let me save some code in the device driver and use
> >> the same paths for both static (DT) and dynamic (configfs)
> >> configuration.
> >> 
> >> Probably I have missed something and I will be greatful, if you tell me
> >> where I can find more information about software nodes. There are few
> >> users in the kernel and it isn't obvious for me how to use software
> >> nodes properly.
> >
> > gpiod_add_lookup_table().
> >
> 
> Yes, that is exactly what my POC code does now. But having a lookup
> table together with the rest of the device structures has several
> advantages.
> 
> 1) The device may be hotpluggable and there is no
>    gpiod_remove_lookup_table().

	% git grep -n -w gpiod_remove_lookup_table

Or I did get it wrong? Did you mean that the removal is not being called?

> 2) Having the lookup table allocated and managed together with the rest
>    of the device seems like a better way to go than on gpio_lookup_list.

Nice, what are you going to do with the rest of lookup tables
(PWM, regulators, etc)? If you convert, convert them all at least.

> 3) As of now I've got a minor issue with device naming. I need to set
>    dev_id of the table before the device is ready and only after it is
>    ready, its name is set (in the hotpluggable use case).

Hotpluggable devices are very much supported by ACPI assistance. DT I have
heard has overlays. What's the issue?

> 4) Because no other devices would use this lookup table "publishing" it
>    rather than keeping together with the device seems at least slightly
>    odd.
> 
> When the lookup table is attached to the devices and can be passed
> around  the final lookup can be done with a function like
> 
> static struct gpio_desc *gpiod_find_from_table(struct device *dev,
>                              const char *con_id, unsigned int idx,
>                  unsigned long *flags, struct gpiod_lookup *table)

Something sounds fishy about your case. Why do you need to have board code /
platform data in the first place? Sorry, but I didn't get why you should
reconstruct DT (or ACPI) at run-time without using proper framework / feature
(overlays)?

> >>>> At the moment the driver gets the list from fwnode/of_node. The list
> >>>> contain references to phandles which get resolved and and the driver
> >>>> ends up with a bunch of gpio descriptors. Great.
> >>>> 
> >>>> This example looks nice but does the code that reads the reference from
> >>>> the gpios property and returns a gpiod actually exist? If it doesn't, I
> >>>> am willing to write it.
> >>>> 
> >>>> At first glance it makes more sense to me to pass (struct gpiod_lookup
> >>>> *) instead of (struct software_node *) and make gpiolib's gpiod_find()
> >>>> accept lookup tables as parameter instead of searching the
> >>>> gpio_lookup_list? Or do you think such temporary table should be
> >>>> assembled from the above structure and then used in gpiod_find()?
> >>>> 
> >>>> Any other suggestions on how to get a bunch of gpios (the description
> >>>> for gpios is available in the devicetree) for a device described with a
> >>>> software nodes?

-- 
With Best Regards,
Andy Shevchenko