Re: [PATCH v6 3/4] of: overlay: add per overlay sysfs attributes

[Pantelis Antoniou]

Hi Rob,

> On Nov 5, 2015, at 21:52 , Rob Herring <robherring2@xxxxxxxxx> wrote:
> 
> On Thu, Oct 22, 2015 at 11:15 AM, Pantelis Antoniou
> <pantelis.antoniou@xxxxxxxxxxxx> wrote:
>> Hi Rob,
>> 
>>> On Oct 22, 2015, at 00:52 , Rob Herring <robherring2@xxxxxxxxx> wrote:
>>> 
>>> On Wed, Oct 21, 2015 at 2:37 PM, Pantelis Antoniou
>>> <pantelis.antoniou@xxxxxxxxxxxx> wrote:
>>>> Hi Rob,
>>>> 
>>>>> On Oct 21, 2015, at 00:54 , Rob Herring <robherring2@xxxxxxxxx> wrote:
>>>>> 
>>>>> On Tue, Oct 20, 2015 at 4:11 PM, Pantelis Antoniou
>>>>> <pantelis.antoniou@xxxxxxxxxxxx> wrote:
>>>>>> Hi Rob,
>>>>>> 
>>>>>>> On Oct 21, 2015, at 00:04 , Rob Herring <robherring2@xxxxxxxxx> wrote:
>>>>>>> 
>>>>>>> On Tue, Oct 20, 2015 at 2:13 PM, Pantelis Antoniou
>>>>>>> <pantelis.antoniou@xxxxxxxxxxxx> wrote:
>>>>>>>> * A per overlay can_remove sysfs attribute that reports whether
>>>>>>>> the overlay can be removed or not due to another overlapping overlay.
>>>>>>>> 
>>>>>>>> * A target sysfs attribute listing the target of each fragment,
>>>>>>>> in a group named after the name of the fragment.
>>>>> 
>>>>> [...]
>>>>> 
>>>>>>>> @@ -255,6 +278,17 @@ err_fail:
>>>>>>>>     return -EINVAL;
>>>>>>>> }
>>>>>>>> 
>>>>>>>> +static ssize_t target_show(struct kobject *kobj,
>>>>>>>> +               struct fragment_attribute *fattr, char *buf)
>>>>>>>> +{
>>>>>>>> +       struct of_overlay_info *ovinfo = fattr->ovinfo;
>>>>>>>> +
>>>>>>>> +       return snprintf(buf, PAGE_SIZE, "%s\n",
>>>>>>>> +                       of_node_full_name(ovinfo->target));
>>>>>>> 
>>>>>>> This can be a link to the node itself, can't it?
>>>>>>> 
>>>>>> 
>>>>>> Yes. Do you want it like this?
>>>>> 
>>>>> Yes, hence the suggestion. Unless you see some reason why not.
>>>>> 
>>>> 
>>>> Nope, canât be done. The sysfs API only allows linking one kobj to another.
>>>> The kobj is the overlay but the target is in the fragment attribute group.
>>> 
>>> Can't we make the fragments kobj's as well?
>>> 
>> 
>> We could, but it break the mental model of what a kobj should represent.
>> An overlay is an object which can be address, a fragment is never directly
>> exposed.
> 
> But you are exposing fragments as there is a directory for each one.
> 
> Overlays are just logical collections of fragments. You could go as
> far to say overlays don't need to be exposed at all and only fragments
> need to be (not that we should). We already have overlays and nodes as
> kobjs, so I don't think fragments as kobjs breaks the mental model.
> 

I disagree; fragments are merely an internal implementation detail, namely that an overlay
is comprised of a collection of fragments.

Being able to address fragments individually, which is what making a fragment a kobj does,
conceptually, is not making sense.

We are being driven to consider having them as kobjs as a consequence of the gaps in the
sysfs internal kernel API, that is that we can only make links between kobjsâ only not in
arbitrary points from one sysfs directory to another.

>> TBH a link attribute is indeed better than a path attribute, but marginally so.
>> Itâs not worth the trouble IMO.
> 
> It is just an ABI we have to support foreverâ
> 

I donât think thatâs that a big deal IMO. Following a link is no different than reading the path attribute.

> Rob

Regards


â Pantelis

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/