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

[Lukasz Stelmach]

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:
>
> ...
>
>> >> I am writing a piece that needs to provide a list of gpios to a
>> >> diriver. The above example looks like what I need.
>> >
>> > Nope.
>> >
>> > It mustn't be used for GPIOs or PWMs or whatever that either should come via
>> > lookup tables or corresponding firmware interface.
>> 
>> May I ask why? I've read commit descriptions for drivers/base/swnode.c
>> and the discussion on lkml and I understand software nodes as a way to
>> provide (synthesize) a description for a device that is missing a
>> description in the firmware. Another use case seems to be to replace (in
>> the long run) platform data. That is what I am trying to use it for.
>
> Yes. Both are correct. They are simply not applicable for everything
> (it's not a silver bullet).
>
>> I want my device to be configured with either DT or software_nodes
>> created at run time with configfs.
>
> Okay.
>
>> My device is going to use GPIOs
>> described in the DT and it is going to be configured via configfs at run
>> time.
>
> How is this related to swnodes?

I thought I should mention it, because as far as I can tel mixing and
merging information from different fwnode backends seems to be tricky if
supported at all.

> 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().

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.

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).

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)

>>>> 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?

-- 
Łukasz Stelmach
Samsung R&D Institute Poland
Samsung Electronics

Attachment: signature.asc
Description: PGP signature