Re: driver model, duplicate names question

[Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>]

On 07/16/2013 12:32 PM, Greg KH wrote:
> On Tue, Jul 16, 2013 at 12:33:03PM -0700, Srinivas Pandruvada wrote:
>>>> +Example Sys-FS Interface
>>>> +
>>>> +/sys/class/power_cap/intel-rapl
>>>> +├── package-0
>>>> +│   ├── constraint-0
>>>> +│   │   ├── name
>>>> +│   │   ├── power_limit_uw
>>>> +│   │   └── time_window_us
>>>> +│   ├── constraint-1
>>>> +│   │   ├── name
>>>> +│   │   ├── power_limit_uw
>>>> +│   │   └── time_window_us
>>>> +│   ├── core
>>>> +│   │   ├── constraint-0
>>>> +│   │   │   ├── name
>>>> +│   │   │   ├── power_limit_uw
>>>> +│   │   │   └── time_window_us
>>>> +│   │   ├── energy_uj
>>>> +│   │   └── max_energy_range_uj
>>>> +│   ├── dram
>>>> +│   │   ├── constraint-0
>>>> +│   │   │   ├── name
>>>> +│   │   │   ├── power_limit_uw
>>>> +│   │   │   └── time_window_us
>>>> +│   │   ├── energy_uj
>>>> +│   │   └── max_energy_range_uj
>>>> +│   ├── energy_uj
>>>> +│   ├── max_energy_range_uj
>>>> +│   └── max_power_range_uw
>>>> +├── package-1
>>>> +│   ├── constraint-0
>>>> +│   │   ├── name
>>>> +│   │   ├── power_limit_uw
>>>> +│   │   └── time_window_us
>>>> +│   ├── constraint-1
>>>> +│   │   ├── name
>>>> +│   │   ├── power_limit_uw
>>>> +│   │   └── time_window_us
>>>> +│   ├── core
>>>> +│   │   ├── constraint-0
>>>> +│   │   │   ├── name
>>>> +│   │   │   ├── power_limit_uw
>>>> +│   │   │   └── time_window_us
>>>> +│   │   ├── energy_uj
>>>> +│   │   └── max_energy_range_uj
>>>> +│   ├── dram
>>>> +│   │   ├── constraint-0
>>>> +│   │   │   ├── name
>>>> +│   │   │   ├── power_limit_uw
>>>> +│   │   │   └── time_window_us
>>>> +│   │   ├── energy_uj
>>>> +│   │   └── max_energy_range_uj
>>>> +│   ├── energy_uj
>>>> +│   ├── max_energy_range_uj
>>>> +│   └── max_power_range_uw
>>>> +├── power
>>>> +│   ├── async
>>>> +│   ├── autosuspend_delay_ms
>>>> +│   ├── control
>>>> +│   ├── runtime_active_kids
>>>> +│   ├── runtime_active_time
>>>> +│   ├── runtime_enabled
>>>> +│   ├── runtime_status
>>>> +│   ├── runtime_suspended_time
>>>> +│   └── runtime_usage
>>>> +├── subsystem -> ../../../../class/power_cap
>>>> +└── uevent
>>> Ick.  Rewrite this to use a bus and you should be fine, right?  Don't
>>> use a class, a class is only to be used if you have a device that is a
>>> specific "type of thing".  Like a tty device, it is a class, as lots of
>>> different "real" devices can have tty ports on them (usb, pci, pcmcia,
>>> platform, etc.)
>>>
>>> Rethink this using a bus and see if that solves your issues.  You get a
>>> hierarchy with that.  And you can have different "types" of devices on
>>> your bus, making it easy to tell the difference between a "package" and
>>> a "constraint".
>>>
>>> Does that help?
>> I will experiment your suggestion. I see this class analogous to
>> "/sys/class/thermal",
>> , where the thermal class provides a set of consistent interface for all
>> thermal devices.
> But thermal devices are not "real" at all.  There are just a number of
> "cooling devices" on a virtual bus and not attached to any type of a
> real device at all.
>
> There's also no hierarchy that I can see with the thermal class, but you
> want to have this, so you will have to do something different because
> classes do not have hierarchies.
>
> So try using a device and a bus and see if that helps out.  If not,
> please let me know.
Experimented by using a device and a bus. As your initial mail pointed 
out, it still fails.
It will try to create symlink to /sys/bus/BUSTYPE/devices/, which will 
prevent duplicate names.
Someone suggested to do like usb by creating file system entries, which 
will require me to
wear a body armour to post upstream for review.

So I think the solution is to use prevent duplicate names. So in the 
above example of sys-fs:

"package-0" may be called power_zone#, with attribute "name" = 
"package_0". Its children can
be called power_zone#:#. I will still use parent child relationships 
during device_register.
For constraints, I will be using attributes like constraint_0_name, 
constraint_0_power_limit etc. under each power_zone.
Hope this is an acceptable solution.
> thanks,
>
> greg k-h
>