Re: [PATCH v2 2/2] soundwire: sysfs: add slave status and device number before probe

From: Vinod Koul <vkoul@kernel.org>
To: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Cc: alsa-devel@alsa-project.org, tiwai@suse.de,
	gregkh@linuxfoundation.org, broonie@kernel.org,
	srinivas.kandagatla@linaro.org,
	Bard liao <yung-chuan.liao@linux.intel.com>,
	Rander Wang <rander.wang@linux.intel.com>
Subject: Re: [PATCH v2 2/2] soundwire: sysfs: add slave status and device number before probe
Date: Wed, 23 Sep 2020 15:33:27 +0530	[thread overview]
Message-ID: <20200923100327.GF2968@vkoul-mobl> (raw)
In-Reply-To: <2ef2e1e8-d7cc-a08f-4176-240750d640a5@linux.intel.com>

On 21-09-20, 09:34, Pierre-Louis Bossart wrote:
> 
> 
> On 9/19/20 6:19 AM, Vinod Koul wrote:
> > On 18-09-20, 09:21, Pierre-Louis Bossart wrote:
> > > 
> > > 
> > > 
> > > > >     * Base file is device
> > > > >     *	|---- modalias
> > > > > + *	|---- dev-status
> > > > > + *		|---- status
> > > > > + *		|---- device_number
> > > > 
> > > > Any reason why we want this under dev-status.
> > > > 
> > > > Both the status and device_number belong to the device, so we can
> > > > put them under device and use device properties
> > > 
> > > We already use directories for device-level and port-level properties, I
> > > just thought it be cleaner to continue this model. We might also expand the
> > > information later on, e.g. provide interrupt status.
> > 
> > Right now we have directories for N ports (needs a dir due to nature of
> > N ports) and 'properties' derived from Disco/firmware.
> > So Nport and properties makes sense. But for generic device level stuff
> > like device number, status and future interrupt or anything should be at
> > device level.
> > 
> > > I don't mind if we remove the directory and move everything up one level,
> > > but it wouldn't be consistent with the previous work.
> > 
> > Just because we had directory for a reason, adding a directory to
> > conform to that does make it better. IMO device files should be at
> > device directory
> 
> We have a "dev-properties" directory, which is added after the driver probe,
> and describes MIPI DisCo values at the device level.

Quite right and the reason to add this after driver probe is to let
driver update the values (hard coded or read from firmware etc)

There is a set of properties which tells us the group of properties
which are coming from MIPI DisCo, so IMO they do belong to a directory

> Either we remove this dev-properties and move it to the device level - to be

It is sysfs ABI it can't be moved, not that I agree with that

> consistent with your recommendation - or we keep separate directories, one
> which is populated on device registration and the other on driver probe.

This is device properties so should not really be a directory!

For example on linux devices we have bunch of files and one specific
directory for power. Here too I would like to see bunch of files and
directory for dp-N and mipi ones

-- 
~Vinod