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

Reply via email to