On Sun, Sep 20, 2020 at 05:03:00PM +0000, Rojewski, Cezary wrote: > On 2020-09-19 4:42 PM, gregkh@xxxxxxxxxxxxxxxxxxx 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. -- With Best Regards, Andy Shevchenko
