On Wed, Apr 07, 2021 at 07:43:44PM +0000, Min Li wrote: > > > > Why do you need 4 files here? Can't you do this all in one? There's no > > need > > for such a small driver to be split up, that just causes added complexity > > and > > makes things harder to review and understand. > > > > We will add more functions and boards down the road. So the abstraction here > is for future consideration
Do not add additional complexity today for stuff that you do not need today. Add it when you need it. > > > include/uapi/linux/rsmu.h | 64 +++++++++++ > > > > Where are you documenting these new custom ioctls? And why do you even > > need them? > > > > Other than comments in the header itself, no additional documenting. Do you > know if Linux has specific place to document custom ioctls? > Renesas software needs to access these ioctls to provide GNSS assisted > partial timing support. More specifically, RSMU_GET_STATE would tell us if a > specific DPLL > is locked to GNSS and RSMU_GET_FFO would tell us how much of frequency offset > for the DPLL to lock to the GNSS. Please provide some sort of documentation and at the least, a pointer to the software that uses this so that we can see how it all ties together. thanks, greg k-h