Re: [PATCH net-next v3 2/2] misc: Add Renesas Synchronization Management Unit (SMU) support

2021-04-08 Thread Greg KH
On Thu, Apr 08, 2021 at 06:59:19PM +, Min Li wrote:
> > 
> > But what does that have to do with the misc device?
> > 
> 
> Hi Greg, MFD driver is the start of everything. Once MFD driver is loading, 
> it will spawn 2 devices,  
> one is for phc driver, which is under /driver/ptp and the other one is for 
> this misc driver.  
> Both PHC and misc drivers are operating on the same device. 
> They are both calling exported functions from mfd drivers to access the 
> device through i2c/spi 
> and the register definitions are located in include/Linux/mfd/idt8a340_reg.h 
> or idt82p33_reg.h
> depending on which device was found by mfd driver through device tree node.

I don't think it's a good idea to have the same defines with different
values in different files, that's just a problem waiting to happen...

good luck!

greg k-h


RE: [PATCH net-next v3 2/2] misc: Add Renesas Synchronization Management Unit (SMU) support

2021-04-08 Thread Min Li
> 
> But what does that have to do with the misc device?
> 

Hi Greg, MFD driver is the start of everything. Once MFD driver is loading, it 
will spawn 2 devices,  
one is for phc driver, which is under /driver/ptp and the other one is for this 
misc driver.  
Both PHC and misc drivers are operating on the same device. 
They are both calling exported functions from mfd drivers to access the device 
through i2c/spi 
and the register definitions are located in include/Linux/mfd/idt8a340_reg.h or 
idt82p33_reg.h
depending on which device was found by mfd driver through device tree node.


Re: [PATCH net-next v3 2/2] misc: Add Renesas Synchronization Management Unit (SMU) support

2021-04-08 Thread Greg KH
On Thu, Apr 08, 2021 at 06:01:42PM +, Min Li wrote:
> > 
> > That does not make sense, this is only one kernel module, with one .h file 
> > in
> > this patch, I do not see those other files you are talking about...
> > 
> > And if you have named registers that are identical, and yet you only work on
> > one device, that feels like a design flaw somewhere :)
> > 
> 
> Hi Greg, the register files are in the 1/2 patch for the mfd part of the 
> change. 
> The reason they have same named register is because they are all 
> synchronization 
> devices and they share some similar features

But what does that have to do with the misc device?

totally confused,

greg k-h


RE: [PATCH net-next v3 2/2] misc: Add Renesas Synchronization Management Unit (SMU) support

2021-04-08 Thread Min Li
> 
> That does not make sense, this is only one kernel module, with one .h file in
> this patch, I do not see those other files you are talking about...
> 
> And if you have named registers that are identical, and yet you only work on
> one device, that feels like a design flaw somewhere :)
> 

Hi Greg, the register files are in the 1/2 patch for the mfd part of the 
change. 
The reason they have same named register is because they are all 
synchronization 
devices and they share some similar features


Re: [PATCH net-next v3 2/2] misc: Add Renesas Synchronization Management Unit (SMU) support

2021-04-08 Thread Greg KH
On Thu, Apr 08, 2021 at 05:26:40PM +, Min Li wrote:
> > 
> > Again, please make this only one file.
> > 
> Hi Greg, the 2 boards have some same named registers in idt82p33_reg.h and 
> idt8a340_reg.h
> so if I put them all in the same file, there will be name conflicts. 

That does not make sense, this is only one kernel module, with one .h
file in this patch, I do not see those other files you are talking
about...

And if you have named registers that are identical, and yet you only
work on one device, that feels like a design flaw somewhere :)

thanks,

greg k-h


RE: [PATCH net-next v3 2/2] misc: Add Renesas Synchronization Management Unit (SMU) support

2021-04-08 Thread Min Li
> 
> Again, please make this only one file.
> 
Hi Greg, the 2 boards have some same named registers in idt82p33_reg.h and 
idt8a340_reg.h
so if I put them all in the same file, there will be name conflicts. 


Re: [PATCH net-next v3 2/2] misc: Add Renesas Synchronization Management Unit (SMU) support

2021-04-07 Thread Greg KH
On Wed, Apr 07, 2021 at 09:50:50PM -0400, min.li...@renesas.com wrote:
> From: Min Li 
> 
> This driver is developed for the IDT ClockMatrix(TM) and 82P33xxx families
> of timing and synchronization devices.It will be used by Renesas PTP Clock
> Manager for Linux (pcm4l) software to provide support to GNSS assisted
> partial timing support (APTS) and other networking timing functions.
> 
> Current version provides kernel API's to support the following functions
> -set combomode to enable SYNCE clock support
> -read dpll's state to determine if the dpll is locked to the GNSS channel
> -read dpll's ffo (fractional frequency offset) in ppqt
> 
> Signed-off-by: Min Li 
> ---
> Change log
> -rebase change to linux-next tree
> -remove uncessary condition checks suggested by Greg
> -fix compile error for x86_64
> -register device through misc_register suggested by Greg
> -change to use module_platform_device to register driver
> -remove empty open and release functions
> -more informational comment for struct rsmu_cdev
> 
>  drivers/misc/Kconfig  |   9 ++
>  drivers/misc/Makefile |   2 +
>  drivers/misc/rsmu_cdev.c  | 231 
> ++
>  drivers/misc/rsmu_cdev.h  |  74 +++
>  drivers/misc/rsmu_cm.c| 166 +
>  drivers/misc/rsmu_sabre.c | 133 ++

Again, please make this only one file.

thanks,

greg k-h