On 03-02-21, 16:44, Vinod Koul wrote: > 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 Reverted and pushed out now -- ~Vinod
