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:
On 2020-09-17 4:12 PM, Cezary Rojewski wrote:
Add sysfs entries for displaying version of FW currently in use as well as binary dump of entire version info, including build and log providers hashes.
...
Greg, would you mind taking a look at these sysfs entries added for new catpt driver (Audio DSP driver for Haswell and Broadwell machines)?
Why me?
Andy (CC'ed) suggested that it's best if sysfs code is routed through you. Given your input, I believe he was right.
'routed' probably is not what I meant, rather 'reviewed'. Because I remember that sysfs got new support for attributes thru certain members of the driver structure. Also I'm not sure I know the policy about binary attributes (it's possible that my knowledge is interfering with sysctl binary attributes that are no-no nowadays).
Anyway, thanks for looking into this!
Link to opening post for the series: [PATCH v6 00/14] ASoC: Intel: Catpt - Lynx and Wildcat point https://www.spinics.net/lists/alsa-devel/msg115765.html
Does lore.kernel.org handle alsa-devel yet?
Believe it does as alsa-devel archive can be found on lore.kernel.org. Not really the guy to answer integration questions, though.
...
- ret = catpt_sysfs_create(cdev);
- if (ret)
goto board_err;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.
It's a member of the struct device_driver, if I'm not mistaken.