On 02-02-21, 10:43, Pierre-Louis Bossart wrote:
On 2/1/21 10:18 PM, Vinod Koul wrote:
On 01-02-21, 10:10, Pierre-Louis Bossart wrote:
On 2/1/21 4:14 AM, Vinod Koul wrote:
On 21-01-21, 17:23, Srinivas Kandagatla wrote:
On 21/01/2021 15:12, Pierre-Louis Bossart wrote:
On 1/21/21 6:03 AM, Srinivas Kandagatla wrote:
I totally agree!
If I understand it correctly in Intel case there will be only one Link ID per bus.
Yes IIUC there would be one link id per bus.
the ida approach gives us unique id for each master,bus I would like to propose using that everywhere
We have cases where link2 is not used but link0, 1 and 3 are. Using the IDA would result in master-0,1,2 being shown, that would throw the integrator off. the link_id is related to hardware and can tolerate gaps, the IDA is typically always increasing and is across the system, not controller specific.
We can debate forever but both pieces of information are useful, so my recommendation is to use both:
snprintf(name, sizeof(name), "master-%d-%d", bus_id, bus->link_id);
I agree we should use both, but does it really make sense for naming? We can keep name in ida and expose the link_id as a parameter for integrators to see in sysfs.
That would mean changing the meaning of sysfs properties:
/*
- The sysfs for properties reflects the MIPI description as given
- in the MIPI DisCo spec
- Base file is:
- sdw-master-N
Key is "The sysfs for properties" is for property files. I am not sure how this implies for a number above. I was thinking of using ID for N here and add a link_id file below which represents the link-id property
|---- revision
|---- clk_stop_modes
|---- max_clk_freq
|---- clk_freq
|---- clk_gears
|---- default_row
|---- default_col
|---- dynamic_shape
|---- err_threshold
*/
N is the link ID in the spec. I am not convinced we'd do the community a service by unilaterally changing what an external spec means, or add a property that's kernel-defined while the rest is supposed to come from firmware. If you want to change the spec then you can contribute feedback in MIPI circles (MIPI have a mechanism for maintainers to provide such feedback without company/employer membership requirements)
So either we add a sysfs layer that represents a controller (better in my opinion so that we can show the link/master count), or keep the existing hierarchy but expand the name with a unique ID so that Qualcomm don't get errors with duplicate sysfs link0 entries.
Anyway we are late in cycle for this.. I am reverting this patch and we can arrive at consensus and fix this for next cycle
Thanks