linux-kernel - Re: [PATCH] soundwire: debugfs: use controller id instead of link

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20210206102242.GM2656@vkoul-mobl.Dlink>
Date:   Sat, 6 Feb 2021 15:52:42 +0530
From:   Vinod Koul <vkoul@...nel.org>
To:     Pierre-Louis Bossart <pierre-louis.bossart@...ux.intel.com>
Cc:     alsa-devel@...a-project.org, gregkh@...uxfoundation.org,
        linux-kernel@...r.kernel.org,
        Srinivas Kandagatla <srinivas.kandagatla@...aro.org>,
        sanyog.r.kale@...el.com, yung-chuan.liao@...ux.intel.com
Subject: Re: [PATCH] soundwire: debugfs: use controller id instead of link_id

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