On 2020-09-21 10:35 AM, gregkh@linuxfoundation.org wrote:
On Sun, Sep 20, 2020 at 05:03:00PM +0000, Rojewski, Cezary wrote:
On 2020-09-19 4:42 PM, gregkh@linuxfoundation.org wrote:
On Fri, Sep 18, 2020 at 03:22:13PM +0000, Rojewski, Cezary wrote:
...
Well, that's just one device driver targeting basically single device available in two flavors. Lack of Documentation/ABI/<sysfs-doc> for solution has been noted though. Will add in v7. As this device is available on /sys/bus/pci0000:00/<dev> is the name for upcoming file: sysfs-bus-pci-devices-catpt ok? Or, would you prevent a different, more explicit one?
Why are you putting random driver-specific attributes on a device owned by a different bus? That can cause problems if you are not careful.
Does the SoC core not provide you with a sound device to do this for instead?
I know not about any device that ASoC core provides for me for such tasks.
And the confusion about ADSP device location stems from incomplete description coming from ACPI tables. I've had a quite lengthy discussion about this with Andy. Re: [PATCH v4 02/13] ASoC: Intel: catpt: Define DSP operations https://www.spinics.net/lists/alsa-devel/msg114651.html
...
Why are you calling a specific function to do all of this? Why not provide a default_groups pointer which allows the driver core to automatically create/destroy the sysfs files for you in a race-free manner with userspace?
That's the recommended way, you should never have to manually create files.
Thanks, that's something new. As this is simple device-driver, I believe you meant usage of sysfs_(add|remove)_group() or their "device" equivalents: device_(add|remove)_group(), is that correct? Haven't found any usage of default_group within /sound/ subsystem what cannot be said about the functions I've just mentioned.
Feel free to correct me if I'm wrong about this.
The bus should provide you with the ability to do this, so look into that for your driver.
But why are you creating a binary sysfs file? That's only for passing data raw through the kernel to/from the hardware, with no parsing/massaging possible. Is that what is happening here?
I guess if there was documentation, it would be easier to review :)
thanks,
greg k-h
In v7 I've removed the binary sysfs file, plus the organization has been simplified - made use of struct device_driver's 'dev_groups' field to automate creation/ removal process of sysfs files as requested. Documentation has too been added so things should be clearer.
Thanks, Czarek