Re: [PATCH misc v2 2/2] misc: Add Renesas Synchronization Management Unit (SMU) support

[Greg KH]

On Thu, Sep 16, 2021 at 05:01:10PM +0000, Min Li wrote:
> 
> 
> > -----Original Message-----
> > From: Greg KH <gregkh@xxxxxxxxxxxxxxxxxxx>
> > Sent: September 16, 2021 12:05 PM
> > To: Min Li <min.li.xe@xxxxxxxxxxx>
> > Cc: arnd@xxxxxxxx; derek.kiernan@xxxxxxxxxx; dragan.cvetic@xxxxxxxxxx;
> > linux-kernel@xxxxxxxxxxxxxxx; lee.jones@xxxxxxxxx
> > Subject: Re: [PATCH misc v2 2/2] misc: Add Renesas Synchronization
> > Management Unit (SMU) support
> > 
> > On Thu, Sep 16, 2021 at 03:54:52PM +0000, Min Li wrote:
> > > >
> > > > Please put that link in the changelog comment and in the .c code as
> > > > well so that people know where to find it.
> > > >
> > > > > >
> > > > > > Why is this new api not a standard one?
> > > > > >
> > > > >
> > > > > There is no actual standard for the GNSS assisted partial timing
> > > > > support (APTS) In terms of Linux kernel API
> > > >
> > > > Then make one!  :)
> > >
> > > Yes it is on our roadmap to do that for next release
> > 
> > Please do it for this kernel api, otherwise you have to support this for the
> > next 20+ years as-is :(
> > 
> 
> In that case, I would have to get back to you in a few months. If you are rejecting this
> change due to this reason. Please tell me explicitly so that I can copy paste to my manager
> and that would be it. Thanks 

Please come up with a unified api that will work with all devices of
this type, and not a one-off ioctl with random structures in it that are
directly tied to the hardware.

> > > > Why not just do this all from userspace then?  You can have spi/i2c
> > > > userspace code, right?  Why does this have to be a kernel driver?
> > > >
> > > We used to do everything in userspace. But since PHC (ptp hardware
> > > clock) came along, we decided to move the driver part to kernel. Please
> > take a look at drivers/ptp/ptp_clockmatrix.c for reference.
> > > Recently, we have some functions like APTS that doesn't belong to PTP
> > > or anything else so we have to split those functions to RSMU misc driver
> > and i2c/spi bus accesses to RSMU MFD driver.
> > 
> > I still do not understand why this has to be a kernel driver, sorry.
> > What exactly forces it to be that way?
> > 
> That is our management decision since everyone is trying to move their driver to Linux kernel
> to contribute so that we don't have to release the driver to each customer separately. The customer
> can just grab the driver from linux

Don't use the kernel as your distribution method for things that should
not belong in the kernel.  There is no need for creating and maintain
kernel modules for drivers that do not need to be drivers at all.  That
feels like you would be doing extra work here.  Writing a simple
userspace library for this that works for all kernel versions would be
much easier.

thanks,

greg k-h