RE: [PATCH net-next v3 1/2] mfd: Add Renesas Synchronization Management Unit (SMU) support
> > Then you need to mention that in the change log. > > What changed from v1 => v2? > Please see my latest [PATCH next v2] and it has the following change log Signed-off-by: Min Li --- -rebase change to linux-next tree -add log to inform driver loading success
RE: [PATCH net-next v3 1/2] mfd: Add Renesas Synchronization Management Unit (SMU) support
> > Then you need to mention that in the change log. > > What changed from v1 => v2? > Please look it up by the latest patch I sent over yesterday. It is under the Signed-off-by --- -rebase change to linux-next tree -add log to inform driver loading success
RE: [PATCH next v2] mfd: Add Renesas Synchronization Management Unit (SMU) support
Hi Lee I just withdraw the misc driver review and sent you a new patch for mfd change alone. I will focus on mfd for now. Please take a look. Thanks Min
RE: [PATCH net-next v2 2/2] misc: Add Renesas Synchronization Management Unit (SMU) support
> > 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. > Hi Greg, I will withdraw this review for misc and focus on MFD part review for now. I will re-submit the misc review after mfd change is in. Thanks Min
RE: [PATCH net-next v3 2/2] misc: Add Renesas Synchronization Management Unit (SMU) support
> > 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
> > 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 1/2] mfd: Add Renesas Synchronization Management Unit (SMU) support
> > If this is v3, there should be a changelog here. > Hi Lee There is no change in v3 for MFD part of code, v3 is only for misc part. But I have to format 2 patches together and that is why mfd part got v3 as well. But it is actually identical to v2
RE: [PATCH net-next v2 2/2] misc: Add Renesas Synchronization Management Unit (SMU) support
> > Why? > Because I need to manipulate name by adding the index to it during run time and use it as miscdev's name snprintf(rsmu->name, sizeof(rsmu->name), "rsmu%d", rsmu->index); rsmu->miscdev.name = rsmu->name; > > Then why are you saving it off? > For things like dev_err(rsmu->dev, "Undefined RSMU IOCTL"); > > Ok, that should be documented a bit, it wasn't obvious :) > Will do for the next patch > > So you can just look it up from the name? > name is based on this index in the first place
RE: [PATCH net-next v3 2/2] misc: Add Renesas Synchronization Management Unit (SMU) support
> > 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 v2 2/2] misc: Add Renesas Synchronization Management Unit (SMU) support
> > Why not use the miscdev name field? > miscdev name field is just a char pointer and I need an static array to manipulate the name with index > > So it's a parent? Why not make this a real platform_device pointer and not > a device pointer? > It is not parent and mfd field is the parent > > What operations? > The MFD driver will create 2 devices, one is for phc driver and another one is this driver. The lock is to make sure these 2 driver's operations are synchronized. > > Index into what? > index is passed by mfd driver and will be used as part of device name such as "rsmu0"
RE: [PATCH net-next v2 2/2] misc: Add Renesas Synchronization Management Unit (SMU) support
>
> Do you really meen "+" here? (sorry, have to ask.)
>
I don't know. All of our Linux kernel code has GPL-2.0+ and I just blindly
inherit it.
> > +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
>
> You should not need this as it's a driver, so you should only use the
> dev_dbg() family of print message functions, no need for pr_*() calls.
>
I have one call to pr_err in rsmu_init
pr_err("Unabled to register %s platform driver", DRIVER_NAME);
RE: [PATCH net-next v2 2/2] misc: Add Renesas Synchronization Management Unit (SMU) support
> > If you do nothing in an open/release function, then there is no need to have > them at all, you can remove them. > > But this feels odd, how do you know what device you are using in your ioctl > command? > The type of board is passed by mfd driver through platform data rsmu->type = pdata->type;
RE: [PATCH net-next v2 2/2] misc: Add Renesas Synchronization Management Unit (SMU) support
> > 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 > > 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. Thanks Min
RE: [PATCH next 2/2] misc: Add Renesas Synchronization Management Unit (SMU) support
> > > > > > Any specific reason you are not using the misc_device api? That > > > would clean up this driver a lot, there's no need to create a whole > > > class just for a single driver. > > > > > > > Hi Greg > > > > No specific reason. I just didn't know the existence of misc_register API. > > Your file is in drivers/misc/ :) > > > Do you recommend using this API to create the device? > > Yes. > > > If yes, can you tell me how to obtain a appropriate MINOR number from > > miscdevice.h? > > No need to reserve one, we don't do that anymore, just ask for a dynamic > value and the next availble one will be given to your driver automatically. > > thanks, > > greg k-h Thanks, I will change to use misc_register and get back to you. Thanks Min
RE: [PATCH next 2/2] misc: Add Renesas Synchronization Management Unit (SMU) support
> > Any specific reason you are not using the misc_device api? That would clean > up this driver a lot, there's no need to create a whole class just for a > single > driver. > Hi Greg No specific reason. I just didn't know the existence of misc_register API. Do you recommend using this API to create the device? If yes, can you tell me how to obtain a appropriate MINOR number from miscdevice.h? Thanks Min
RE: [PATCH next 2/2] misc: Add Renesas Synchronization Management Unit (SMU) support
> > Then the patches are independant and they should be sent as such, > otherwise it causes confusion and our tools get messed up when trying to > grab the whole "series" of patches. > > Can you please fix this up and just send two independant patches? > Hi Greg These 2 patches are not independent. Patch 2/2 depends on patch 1/2 to build and work successfully. They just belong to different domain, patch 1/2 belongs to MFD while patch 2/2 belongs to MISC. That is why I wasn't sure if I should send both patches to you in the first place. Anyways, I just sent both patches again. Thanks Min
RE: [PATCH next 2/2] misc: Add Renesas Synchronization Management Unit (SMU) support
> > Where is patch 1/2 of this series? > > Also, please fix up the errors that the testing bot found, and properly > version > your patch submission so I know which one is the "latest" one to look at. > Hi Greg The first patch is mfd so I was not sure if I should send that to you guys in the first place. Anyways, I just sent it over. Min
RE: [PATCH mfd v1] mfd: Add Renesas Synchronization Management Unit (SMU) support
> > > > I am wondering if this is the correct tree to submit the patch for MFD? > > > > So to sum it up, the latest patch is my first version to this tree. > > Either MFD or -next is fine for MFD-only patches. > > Has the code changed at all in any of the patches? > > If so, please provide a change-log, so we can keep up. > > -- I kind of want to discard the previous patch and start a brand new review based on next, can I do that?
RE: [PATCH mfd v1] mfd: Add Renesas Synchronization Management Unit (SMU) support
> > I'm pretty confused. This has been sent ~6 times already. What is the v1 of? > Is this a different driver? If so, why does it have the same $SUBJECT line? > > If this is not actually v1. Please provide a change-log. > Hi Lee Sorry for confusion. This is no version before v1. The reason you saw it multiple times is because I was basing the change in the wrong tree and I now I switched it to https://git.kernel.org/pub/scm/linux/kernel/git/lee/mfd.git/ I am wondering if this is the correct tree to submit the patch for MFD? So to sum it up, the latest patch is my first version to this tree. Min
RE: [PATCH net-next v1] misc: Add Renesas Synchronization Management Unit (SMU) support
Hi Greg You are right, this is not for the network tree. Can you point me to the right tree to base this change for? Thanks Min > -Original Message- > From: Greg KH > Sent: March 17, 2021 3:20 PM > To: Min Li > Cc: derek.kier...@xilinx.com; dragan.cve...@xilinx.com; a...@arndb.de; > linux-kernel@vger.kernel.org; linux-...@vger.kernel.org > Subject: Re: [PATCH net-next v1] misc: Add Renesas Synchronization > Management Unit (SMU) support > > On Wed, Mar 17, 2021 at 02:59:56PM -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) > > > > Signed-off-by: Min Li > > Why the "net-next" in the subject line? > > This is not for the network tree (given that you didn't even cc: > netdev...) or am I confused? > > thanks, > > gre gk-h
RE: [PATCH net-next] misc: Add Renesas Synchronization Management Unit (SMU) support
Hi Arnd This is Min again. I just got off the meeting with our Architect and I would like to correct some confusing information that I delivered before I said this driver supports 1588/PTP related functionalities. This is not correct. In fact, this driver is developed to support our "GNSS Assisted Partial Timing Support" feature, which is not in PTP domain. So I would categorize Renesas SMU (synchronization management unit) device as a multi-functional device and we are actually having a separate review with Lee Jones for the MFD driver alone. Above the MFD driver, we have PHC driver to support PTP and this driver for miscellaneous functions like GNSS APTS stuff. I also attached the diagram drawing, where left column is the old way we are doing things and right column is how we wanna do it through MFD now. Hopefully, this will help you reconsider my proposal. Please do not hesitate to get back to me for this issue. Thanks Min > -Original Message- > From: Arnd Bergmann > Sent: February 18, 2021 5:51 AM > To: Min Li > Cc: Derek Kiernan ; Dragan Cvetic > ; Arnd Bergmann ; gregkh > ; linux-kernel@vger.kernel.org; Networking > ; Richard Cochran > Subject: Re: [PATCH net-next] misc: Add Renesas Synchronization > Management Unit (SMU) support > > On Thu, Feb 18, 2021 at 4:28 AM Min Li wrote: > > > If the driver can use the same algorithm that is in your user space > > > software today, that would seem to be a nicer way to handle it than > > > requiring a separate application. > > > > > > > Hi Arnd > > > > > > What is the device driver that you are referring here? > > > > In summary of your reviews, are you suggesting me to discard this > > change and go back to PTP subsystem to find a better place for things > > that I wanna do here? > > Yes, I mean doing it all in the PTP driver. > > Arnd rsmu.pdf Description: rsmu.pdf
RE: [PATCH net-next] mfd: Add Renesas Synchronization Management Unit (SMU) support
Hi Lee
When you have time, can you please take a look at my review below?
Thanks
Min
> -Original Message-
> From: min.li...@renesas.com
> Sent: February 10, 2021 10:00 PM
> To: lee.jo...@linaro.org
> Cc: linux-kernel@vger.kernel.org; Min Li
> Subject: [PATCH net-next] mfd: Add Renesas Synchronization Management
> Unit (SMU) support
>
> From: Min Li
>
> Add support for ClockMatrix(TM) and 82P33xxx families of timing and
> synchronization devices. Currently only I2C access is supported and SPI would
> be added later on.
>
> This mfd driver will export 2 devices to be used by rsmu_cdev and the
> corresponding phc drivers, respectively.
>
> Signed-off-by: Min Li
> ---
> drivers/mfd/Kconfig | 18 +
> drivers/mfd/Makefile | 1 +
> drivers/mfd/rsmu_i2c.c | 483 +++
> drivers/mfd/rsmu_private.h | 49 +++
> include/linux/mfd/idt82p33_reg.h | 102 +
> include/linux/mfd/idt8a340_reg.h | 807
> +++
> include/linux/mfd/rsmu.h | 42 ++
> 7 files changed, 1502 insertions(+)
> create mode 100644 drivers/mfd/rsmu_i2c.c create mode 100644
> drivers/mfd/rsmu_private.h create mode 100644
> include/linux/mfd/idt82p33_reg.h create mode 100644
> include/linux/mfd/idt8a340_reg.h create mode 100644
> include/linux/mfd/rsmu.h
>
> diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig index
> bdfce7b..ea8aa688 100644
> --- a/drivers/mfd/Kconfig
> +++ b/drivers/mfd/Kconfig
> @@ -2085,6 +2085,24 @@ config MFD_KHADAS_MCU
> additional drivers must be enabled in order to use the functionality
> of the device.
>
> +config MFD_RSMU_I2C
> + tristate "Support for Renesas Synchronization Management Unit
> (SMU)"
> + depends on I2C && OF
> + select MFD_CORE
> + help
> + Select this option to enable chip driver for Renesas SMU, such as
> + Clockmatrix and 82P33XXX series. This option supports I2C as
> + the control interface.
> +
> +config RSMU_USE_REGMAP
> + bool "Use regmap to access RSMU through I2C/SPI bus"
> + depends on MFD_RSMU_I2C
> + select REGMAP_I2C
> + select REGMAP_SPI
> + default n
> + help
> + Please choose no for Clockmatrix series SMU.
> +
> menu "Multimedia Capabilities Port drivers"
> depends on ARCH_SA1100
>
> diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile index
> 14fdb18..137f125 100644
> --- a/drivers/mfd/Makefile
> +++ b/drivers/mfd/Makefile
> @@ -238,6 +238,7 @@ obj-$(CONFIG_MFD_HI655X_PMIC) += hi655x-
> pmic.o
> obj-$(CONFIG_MFD_DLN2) += dln2.o
> obj-$(CONFIG_MFD_RT5033) += rt5033.o
> obj-$(CONFIG_MFD_SKY81452) += sky81452.o
> +obj-$(CONFIG_MFD_RSMU_I2C) += rsmu_i2c.o
>
> intel-soc-pmic-objs := intel_soc_pmic_core.o
> intel_soc_pmic_crc.o
> obj-$(CONFIG_INTEL_SOC_PMIC) += intel-soc-pmic.o
> diff --git a/drivers/mfd/rsmu_i2c.c b/drivers/mfd/rsmu_i2c.c new file mode
> 100644 index 000..5164a45
> --- /dev/null
> +++ b/drivers/mfd/rsmu_i2c.c
> @@ -0,0 +1,483 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Multi-function driver for the IDT ClockMatrix(TM) and 82P33xxx
> +families of
> + * timing and synchronization devices.
> + *
> + * Copyright (C) 2019 Integrated Device Technology, Inc., a Renesas
> Company.
> + */
> +
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +
> +#include "rsmu_private.h"
> +
> +
> +#ifdef CONFIG_RSMU_USE_REGMAP
> +static bool rsmu_cm_volatile_reg(struct device *dev, unsigned int reg);
> +static bool rsmu_sabre_volatile_reg(struct device *dev, unsigned int
> +reg); #endif
> +
> +/* Current mfd device index */
> +static atomic_t rsmu_ndevs = ATOMIC_INIT(0);
> +
> +/* SMU page registers */
> +static u8 rsmu_page_regs[] = {
> + [RSMU_CM] = RSMU_CM_PAGE_ADDR,
> + [RSMU_SABRE] = RSMU_SABRE_PAGE_ADDR,
> +};
> +
> +/* Platform data */
> +static struct rsmu_pdata rsmu_pdata[RSMU_MAX_MFD_DEV];
> +
> +/* clockmatrix phc devices */
> +static struct mfd_cell rsmu_cm_pdev[RSMU_MAX_MFD_DEV] = {
> + [0] = {
> + .name = "idtcm-ptp0",
> + .of_compatible = "renesas,idtcm-ptp0",
> + },
> + [1] = {
> + .name = "idtcm-ptp1",
> + .of_compatible = "renesas,idtcm-ptp1",
> + },
> + [2] = {
> + .name = "idtcm-ptp2",
> + .of_compatible = "renesas
FW: [PATCH net-next] misc: Add Renesas Synchronization Management Unit (SMU) support
-Original Message- From: Min Li Sent: February 18, 2021 11:14 AM To: 'Arnd Bergmann' Cc: Derek Kiernan ; Dragan Cvetic ; Arnd Bergmann ; gregkh ; linux-kernel@vger.kernel.org; Networking ; Richard Cochran Subject: RE: [PATCH net-next] misc: Add Renesas Synchronization Management Unit (SMU) support > -Original Message- > From: Arnd Bergmann > Sent: February 18, 2021 5:51 AM > To: Min Li > Cc: Derek Kiernan ; Dragan Cvetic > ; Arnd Bergmann ; gregkh > ; linux-kernel@vger.kernel.org; Networking > ; Richard Cochran > Subject: Re: [PATCH net-next] misc: Add Renesas Synchronization > Management Unit (SMU) support > > On Thu, Feb 18, 2021 at 4:28 AM Min Li wrote: > > > If the driver can use the same algorithm that is in your user > > > space software today, that would seem to be a nicer way to handle > > > it than requiring a separate application. > > > > > > > Hi Arnd > > > > > > What is the device driver that you are referring here? > > > > In summary of your reviews, are you suggesting me to discard this > > change and go back to PTP subsystem to find a better place for > > things that I wanna do here? > > Yes, I mean doing it all in the PTP driver. > > Arnd Hi Arnd The APIs I am adding here is for our development of assisted partial timing support (APTS), which is a Global Navigation Satellite System (GNSS) backed by Precision Time Protocol (PTP). So it is not part of PTP but they can work together for network timing solution. What I am trying to say is the things that I am adding here doesn't really belong to the PTP world. For example, timex->freq is different from the ffo that I am reading from this driver since the DPLL is Working in different mode. For PTP, DPLL is working in DCO mode. In DCO mode, the DPLL control loop is opened and the DCO can be controlled by a PTP clock recovery servo running on an external processor to synthesize PTP clocks. On the other hand for GNSS timing, the ffo I am reading here is when DPLL is in locked mode. In Locked the long-term output frequency accuracy is the same as the long-term frequency accuracy of the selected input reference. For our GNSS APTS development, we have 2 DPLL channels, one channel is locked to GNSS and another channel is PTP channel. If GNSS channel is locked, we use GNSS's channel to support network timing. Otherwise, we switch to PTP channel. To think about it, our device is really an multi functional device (MFD), which is why I am submitting another review for our MFD driver on the side. We have our PTP driver and we have this for GNSS APTS and other misc functions. So can you take a look at this again and see if it makes sense to keep this change simply because the change is not part of PTP subsystem. They sound like they are related. But when it comes to technicality, there is really no place in PTP to hold stuff that I am doing here. Thanks Min
RE: [PATCH net-next] misc: Add Renesas Synchronization Management Unit (SMU) support
> -Original Message- > From: Arnd Bergmann > Sent: February 18, 2021 5:51 AM > To: Min Li > Cc: Derek Kiernan ; Dragan Cvetic > ; Arnd Bergmann ; gregkh > ; linux-kernel@vger.kernel.org; Networking > ; Richard Cochran > Subject: Re: [PATCH net-next] misc: Add Renesas Synchronization > Management Unit (SMU) support > > On Thu, Feb 18, 2021 at 4:28 AM Min Li wrote: > > > If the driver can use the same algorithm that is in your user space > > > software today, that would seem to be a nicer way to handle it than > > > requiring a separate application. > > > > > > > Hi Arnd > > > > > > What is the device driver that you are referring here? > > > > In summary of your reviews, are you suggesting me to discard this > > change and go back to PTP subsystem to find a better place for things > > that I wanna do here? > > Yes, I mean doing it all in the PTP driver. > > Arnd Hi Arnd The APIs I am adding here is for our development of assisted partial timing support (APTS), which is a Global Navigation Satellite System (GNSS) backed by Precision Time Protocol (PTP). So it is not part of PTP but they can work together for network timing solution. What I am trying to say is the things that I am adding here doesn't really belong to the PTP world. For example, timex->freq is different from the ffo that I am reading from this driver since the DPLL is Working in different mode. For PTP, DPLL is working in DCO mode. In DCO mode, the DPLL control loop is opened and the DCO can be controlled by a PTP clock recovery servo running on an external processor to synthesize PTP clocks. On the other hand for GNSS timing, the ffo I am reading here is when DPLL is in locked mode. In Locked the long-term output frequency accuracy is the same as the long-term frequency accuracy of the selected input reference. For our GNSS APTS development, we have 2 DPLL channels, one channel is locked to GNSS and another channel is PTP channel. If GNSS channel is locked, we use GNSS's channel to support network timing. Otherwise, we switch to PTP channel. To think about it, our device is really an multi functional device (MFD), which is why I am submitting another review for our MFD driver on the side. We have our PTP driver and we have this for GNSS APTS and other misc functions. So can you take a look at this again and see if it makes sense to keep this change simply because the change is not part of PTP subsystem. They sound like they are related. But when it comes to technicality, there is really no place in PTP to hold stuff that I am doing here. Thanks Min
RE: [PATCH net-next] misc: Add Renesas Synchronization Management Unit (SMU) support
> -Original Message- > From: Arnd Bergmann > Sent: February 17, 2021 4:30 PM > To: Min Li > Cc: Derek Kiernan ; Dragan Cvetic > ; Arnd Bergmann ; gregkh > ; linux-kernel@vger.kernel.org; Networking > ; Richard Cochran > Subject: Re: [PATCH net-next] misc: Add Renesas Synchronization > Management Unit (SMU) support > > On Wed, Feb 17, 2021 at 9:20 PM Min Li wrote: > > > > I attached the G.8273.2 document, where chapter 6 is about supporting > > physical layer frequency. And combo mode is Renesas way to support > > this requirement. Other companies may come up with different ways to > support it. > > > > When EEC quality is below certain level, we would wanna turn off combo > mode. > > Maybe this is something that could be handled inside of the device driver > then? > > If the driver can use the same algorithm that is in your user space software > today, that would seem to be a nicer way to handle it than requiring a > separate application. > Hi Arnd What is the device driver that you are referring here? In summary of your reviews, are you suggesting me to discard this change and go back to PTP subsystem to find a better place for things that I wanna do here? Min
RE: [PATCH net-next] misc: Add Renesas Synchronization Management Unit (SMU) support
> > I can't help but think you are evading my question I asked. If there is no > specific action that this pcm4l tool needs to perform, then I'd think we > should better not provide any interface for it at all. > > I also found a reference to only closed source software at > https://www.renesas.com/us/en/software-tool/ptp-clock-manager-linux > We don't add low-level interfaces to the kernel that are only usable by > closed-source software. > > Once you are able to describe the requirements for what pcm4l actually > needs from the hardware, we can start discussing what a high-level > interface would look like that can be used to replace the your current > interface, in a way that would work across vendors and with both pcm4l and > open-source tools that do the same job. > > Arnd Hi Arnd This driver is used by pcm4l to access functionalities that cannot be accessed through PHC(ptp hardware clock) interface. All these functions are kind of specific to Renesas SMU device and I have never heard other devices offering similar functions The 3 functions currently provided are (more to be added in the future) - set combomode In Telecom Boundary Clock (T-BC) and Telecom Time Slave Clock (T-TSC) applications per ITU-T G.8275.2, two DPLLs can be used: one DPLL is configured as a DCO to synthesize PTP clocks, and the other DPLL is configured as an EEC(Ethernet Equipment Clock) to generate physical layer clocks. Combo mode provides physical layer frequency support from the EEC/SEC to the PTP clock. - read DPLL's FFO Read fractional frequency offset (FFO) from a DPLL. For a DPLL channel, a Frequency Control Word (FCW) is used to adjust the frequency output of the DCO. A positive value will increase the output frequency and a negative one will decrease the output frequency. This function will read FCW first and convert it to FFO. -read DPLL's state The DPLLs support four primary operating modes: Free-Run, Locked, Holdover, and DCO. In Free-Run mode the DPLLs synthesize clocks based on the system clock alone. In Locked mode the DPLLs filter reference clock jitter with the selected bandwidth. Additionally in Locked mode, the long-term output frequency accuracy is the same as the long-term frequency accuracy of the selected input reference. In Holdover mode, the DPLL uses frequency data acquired while in Locked mode to generate accurate frequencies when input references are not available. In DCO mode, the DPLL control loop is opened and the DCO can be controlled by a PTP clock recovery servo running on an external processor to synthesize PTP clocks. Again, at the bottom, these function are just reading/writing certain registers through I2C/SPI interface. I am making this driver to support pcm4l since my my company, Renesas, wants to abstract hw details into the kernel. But I can not figure out how to make this universally applied interface and I find misc is the best place to hold driver like this. On the other hand, if you have better ideas, I am all ears. Min
RE: [PATCH net-next v2] misc: Add Renesas Synchronization Management Unit (SMU) support
> > Yes, that line. > > The documentation should tell you how to do that, as per my patch bot: > > - This looks like a new version of a previously submitted patch, but you > did not list below the --- line any changes from the previous version. > Please read the section entitled "The canonical patch format" in the > kernel file, Documentation/SubmittingPatches for what needs to be done > here to properly describe this. > > thanks, > > greg k-h Hi Greg I read documentation. Am I supposed to add the "---" marker line manually by myself when doing "git commit --amend"? git commit will always add a "---" marker line before the actual change like below Signed-off-by: Min Li --- Changes since v1: -Provide more background for purpose of the change. -Provide compat_ioctl support -Fix ioctl cmd definition --- drivers/misc/Kconfig | 13 ++ drivers/misc/Makefile | 3 +
RE: [PATCH net-next] misc: Add Renesas Synchronization Management Unit (SMU) support
> > > > If I come up with a new file and move all the abstraction code there, > > does that work? > > I think so, but it's more important to figure out a good user space interface > first. The ioctl interfaces should be written on a higher-level abstraction, > to > ensure they can work with any hardware implementation and are not > specific to Renesas devices. > > Can you describe on an abstract level how a user would use the character > device, and what they achieve by that? > >Arnd Hi Arnd This driver is meant to be used by Renesas PTP Clock Manager for Linux (pcm4l) software for Renesas device only. About how pcm4l uses the char device, pcm4l will open the device and do the supported ioctl cmds on the device, simple like that. At the same time, pcm4l will also open ptp hardware clock device, which is /dev/ptp[x], to do clock adjustments. Min
RE: [PATCH net-next v2] misc: Add Renesas Synchronization Management Unit (SMU) support
> > On Fri, Feb 12, 2021 at 03:44:52PM +0000, Min Li wrote: > > > > > > > > -set combomode > > > > -get dpll's state > > > > -get dpll's ffo > > > > > > > > This driver must work with Renesas MFD driver to access SMU > > > > through I2C/SPI. > > > > > > > > Changes since v1: > > > > -Provide more background for purpose of the change. > > > > -Provide compat_ioctl support > > > > -Fix ioctl cmd definition > > > > > > This "changes" list goes below the --- line. > > > > > > > Sorry, is this a problem that you are requesting me to address? I am bit > confused... > > Yes, please place that list of changes below the --- line in your patch. > The documentation says to do this, right? > Hi Greg Do you mean this "---" like below? How can I do that? Sorry I was never asked to do that from other reviewer. Signed-off-by: Min Li --- drivers/misc/Kconfig | 13 ++ drivers/misc/Makefile | 3 + drivers/misc/rsmu_cdev.c | 449 ++ drivers/misc/rsmu_cdev.h | 75 drivers/misc/rsmu_cm.c| 214 ++ drivers/misc/rsmu_sabre.c | 153 include/uapi/linux/rsmu.h | 61 +++ 7 files changed, 968 insertions(+) create mode 100644 drivers/misc/rsmu_cdev.c create mode 100644 drivers/misc/rsmu_cdev.h create mode 100644 drivers/misc/rsmu_cm.c create mode 100644 drivers/misc/rsmu_sabre.c create mode 100644 include/uapi/linux/rsmu.h
RE: [PATCH net-next] misc: Add Renesas Synchronization Management Unit (SMU) support
> > Ah, so if this is for a PTP related driver, it should probably be integrated > into > the PTP subsystem rather than being a separate class. > I was trying to add these functions to PHC subsystem but was not accepted because the functions are specific to Renesas device and there is no place for those functions in PHC driver. > > > A pure list of register values seems neither particular portable nor > intuitive. > > > How is a user expected to interpret these, and are you sure that any > > > driver for this class would have the same interpretation at the same > register index? > > > > > > > Yes we need a way to dump register values when remote debugging with > customers. > > And all the Renesas SMU has similar register layout > > A sysfs interface is a poor choice for this though -- how can you guarantee > that even future Renesas devices follow the exact same register layout? By > encoding the current hardware generation into the user interface, you would > end up having to emulate this on other chips you want to support later. > > If it's only for debugging, best leave it out of the public interface, and > only > have it in your own copy of the driver until the bugs are gone, or add a > debugfs interface. > I will drop the sysfs change in the new patch > > Unit tests are good, but it's better to have them in the kernel. > Can you add the unit test into the patch then? > We now have the kunit framework for running unit tests. > Our unit test is based on ceedling. But I will definitely look into the kunit and try to Transfer it to kunit for the next release. > > > This tells me that you got the abstraction the wrong way: the common > > > files should not need to know anything about the specific > implementations. > > > > > > Instead, these should be in separate modules that call exported > > > functions from the common code. > > > > > > > > > > I got what you mean. But so far it only supports small set of > > functions, which is why I don't feet it is worth the effort to over abstract > things. > > Then maybe pick one of the two hardware variants and drop the abstraction > you have. You can then add more features before you add a proper > abstraction layer and then the second driver. > If I come up with a new file and move all the abstraction code there, does that work?
RE: [PATCH net-next v2] misc: Add Renesas Synchronization Management Unit (SMU) support
> > > > If I come up with an rsmu.rst under Documentation/driver-api, is that > something you are looking for? > > No, all sysfs files need to be documented in Documentation/ABI/ > > thanks, > > greg k-h Hi Greg I decided to follow Arnd's suggestion and drop the extra sysfs attributes declaration here. Any other documents that you are looking for here?
RE: [PATCH net-next v2] misc: Add Renesas Synchronization Management Unit (SMU) support
>
> xilinx_sdfec.c has:
>
> static int xsdfec_dev_open(struct inode *iptr, struct file *fptr)
> {
> return 0;
> }
>
> Which isn't even needed at all, but it is NOT trying to keep people from
> calling open multiple times.
>
> As for why the above logic does not work in your driver, think of what
> happens if someone opens the character device node, and then calls
> dup(2) on it and passes that file descriptor off to another program. Or just
> calls it multiple times from different threads in the same program.
> The kernel does not know what is happening here, and so, "do not allow to
> be opened multiple times" does not do anything to keep userspace from
> actually writing to the device node from multiple processes or threads.
>
> So don't even try, it's not worth it.
>
> > I mean if an application failed at opening the device, how can it
> > proceed to talk the device without a file descriptor?
>
> See above for how to do this.
>
> thanks,
>
> greg k-h
Hi Greg
Thanks for your insight for this. Now I can see this change doesn't prevent
deliberate hacker from opening the driver multiple times.
What I had in mind is that it does prevent some unintentional mistake like some
one accidentally opens the application twice. The second
one would fail due to the change here.
RE: [PATCH net-next v2] misc: Add Renesas Synchronization Management Unit (SMU) support
> > > > -set combomode > > -get dpll's state > > -get dpll's ffo > > > > This driver must work with Renesas MFD driver to access SMU through > > I2C/SPI. > > > > Changes since v1: > > -Provide more background for purpose of the change. > > -Provide compat_ioctl support > > -Fix ioctl cmd definition > > This "changes" list goes below the --- line. > Sorry, is this a problem that you are requesting me to address? I am bit confused... > And you seem to have ignored my review, especially about documenting the > sysfs files here, please do that as I can not accept this patch as-is. > If I come up with an rsmu.rst under Documentation/driver-api, is that something you are looking for?
RE: [PATCH net-next v2] misc: Add Renesas Synchronization Management Unit (SMU) support
> > +
> > + /* Only one open per device at a time */
> > + if (!atomic_dec_and_test(&rsmu->open_count)) {
> > + atomic_inc(&rsmu->open_count);
> > + return -EBUSY;
>
> This does not do what you think it does, and does not prevent multiple
> applications from talking to your device at the same time.
>
> There is no need for this at all, as it does not work, sorry. If multiple
> apps
> talk to your device, it's their fault, not the kernel's fault, that things go
> wrong.
>
> And I thought that Arnd already told you to fix this?
>
Hi Greg
Sorry for not replying to the list, I am new so not very familiar with the
process.
Can you elaborate why it doesn't work? I kind of borrow the idea from
xilinx_sdfec.c and I don't see why it doesn't work.
I mean if an application failed at opening the device, how can it proceed to
talk the device without a file descriptor?
Thanks
Min
RE: [PATCH net 3/4] ptp: ptp_idt82p33: use do_aux_work for delay work
Hi David/Richard
When you have time, can you take a look at this change? Thanks
Min
-Original Message-
From: min.li...@renesas.com
Sent: August 7, 2020 11:58 AM
To: richardcoch...@gmail.com
Cc: net...@vger.kernel.org; linux-kernel@vger.kernel.org; Min Li
Subject: [PATCH net 3/4] ptp: ptp_idt82p33: use do_aux_work for delay work
From: Min Li
Instead of declaring its own delay_work, use ptp_clock provided do_aux_work to
configure sync_tod.
Signed-off-by: Min Li
---
drivers/ptp/ptp_idt82p33.c | 24
drivers/ptp/ptp_idt82p33.h | 2 --
2 files changed, 12 insertions(+), 14 deletions(-)
diff --git a/drivers/ptp/ptp_idt82p33.c b/drivers/ptp/ptp_idt82p33.c index
189bb81..2d62aed 100644
--- a/drivers/ptp/ptp_idt82p33.c
+++ b/drivers/ptp/ptp_idt82p33.c
@@ -531,8 +531,8 @@ static int idt82p33_sync_tod(struct idt82p33_channel
*channel, bool enable)
if (enable == channel->sync_tod_on) {
if (enable && sync_tod_timeout) {
- mod_delayed_work(system_wq, &channel->sync_tod_work,
-sync_tod_timeout * HZ);
+ ptp_schedule_worker(channel->ptp_clock,
+ sync_tod_timeout * HZ);
}
return 0;
}
@@ -555,24 +555,27 @@ static int idt82p33_sync_tod(struct idt82p33_channel
*channel, bool enable)
channel->sync_tod_on = enable;
if (enable && sync_tod_timeout) {
- mod_delayed_work(system_wq, &channel->sync_tod_work,
-sync_tod_timeout * HZ);
+ ptp_schedule_worker(channel->ptp_clock,
+ sync_tod_timeout * HZ);
}
return 0;
}
-static void idt82p33_sync_tod_work_handler(struct work_struct *work)
+static long idt82p33_sync_tod_work_handler(struct ptp_clock_info *ptp)
{
struct idt82p33_channel *channel =
- container_of(work, struct idt82p33_channel, sync_tod_work.work);
+ container_of(ptp, struct idt82p33_channel, caps);
struct idt82p33 *idt82p33 = channel->idt82p33;
+ int ret;
mutex_lock(&idt82p33->reg_lock);
- (void)idt82p33_sync_tod(channel, false);
+ ret = idt82p33_sync_tod(channel, false);
mutex_unlock(&idt82p33->reg_lock);
+
+ return ret;
}
static int idt82p33_pps_enable(struct idt82p33_channel *channel, bool enable)
@@ -659,10 +662,8 @@ static void idt82p33_ptp_clock_unregister_all(struct
idt82p33 *idt82p33)
channel = &idt82p33->channel[i];
- if (channel->ptp_clock) {
+ if (channel->ptp_clock)
ptp_clock_unregister(channel->ptp_clock);
- cancel_delayed_work_sync(&channel->sync_tod_work);
- }
}
}
@@ -862,8 +863,6 @@ static int idt82p33_channel_init(struct idt82p33_channel
*channel, int index)
return -EINVAL;
}
- INIT_DELAYED_WORK(&channel->sync_tod_work,
- idt82p33_sync_tod_work_handler);
channel->sync_tod_on = false;
channel->current_freq_ppb = 0;
@@ -881,6 +880,7 @@ static void idt82p33_caps_init(struct ptp_clock_info *caps)
caps->gettime64 = idt82p33_gettime;
caps->settime64 = idt82p33_settime;
caps->enable = idt82p33_enable;
+ caps->do_aux_work = idt82p33_sync_tod_work_handler;
}
static int idt82p33_enable_channel(struct idt82p33 *idt82p33, u32 index) diff
--git a/drivers/ptp/ptp_idt82p33.h b/drivers/ptp/ptp_idt82p33.h index
9d46966..1dcd2c0 100644
--- a/drivers/ptp/ptp_idt82p33.h
+++ b/drivers/ptp/ptp_idt82p33.h
@@ -119,8 +119,6 @@ struct idt82p33_channel {
struct ptp_clock*ptp_clock;
struct idt82p33 *idt82p33;
enum pll_mode pll_mode;
- /* task to turn off SYNC_TOD bit after pps sync */
- struct delayed_work sync_tod_work;
boolsync_tod_on;
s32 current_freq_ppb;
u8 output_mask;
--
2.7.4
RE: [PATCH net 2/4] ptp: ptp_idt82p33: add more debug logs
Hi David/Richard
When you have time, can you take a look at this change? Thanks
Min
-Original Message-
From: min.li...@renesas.com
Sent: August 7, 2020 11:58 AM
To: richardcoch...@gmail.com
Cc: net...@vger.kernel.org; linux-kernel@vger.kernel.org; Min Li
Subject: [PATCH net 2/4] ptp: ptp_idt82p33: add more debug logs
From: Min Li
Signed-off-by: Min Li
---
drivers/ptp/ptp_idt82p33.c | 88 +-
1 file changed, 79 insertions(+), 9 deletions(-)
diff --git a/drivers/ptp/ptp_idt82p33.c b/drivers/ptp/ptp_idt82p33.c index
bd1fbcd..189bb81 100644
--- a/drivers/ptp/ptp_idt82p33.c
+++ b/drivers/ptp/ptp_idt82p33.c
@@ -86,6 +86,7 @@ static int idt82p33_xfer(struct idt82p33 *idt82p33,
struct i2c_client *client = idt82p33->client;
struct i2c_msg msg[2];
int cnt;
+ char *fmt = "i2c_transfer failed at %d in %s for %s, at addr:
+%04X!\n";
msg[0].addr = client->addr;
msg[0].flags = 0;
@@ -99,7 +100,12 @@ static int idt82p33_xfer(struct idt82p33 *idt82p33,
cnt = i2c_transfer(client->adapter, msg, 2);
if (cnt < 0) {
- dev_err(&client->dev, "i2c_transfer returned %d\n", cnt);
+ dev_err(&client->dev,
+ fmt,
+ __LINE__,
+ __func__,
+ write ? "write" : "read",
+ (u8) regaddr);
return cnt;
} else if (cnt != 2) {
dev_err(&client->dev,
@@ -448,8 +454,13 @@ static int idt82p33_measure_tod_write_overhead(struct
idt82p33_channel *channel)
err = idt82p33_measure_settime_gettime_gap_overhead(channel, &gap_ns);
- if (err)
+ if (err) {
+ dev_err(&idt82p33->client->dev,
+ "Failed at line %d in func %s!\n",
+ __LINE__,
+ __func__);
return err;
+ }
err = idt82p33_measure_one_byte_write_overhead(channel,
&one_byte_write_ns);
@@ -613,13 +624,23 @@ static int idt82p33_enable_tod(struct idt82p33_channel
*channel)
err = idt82p33_pps_enable(channel, false);
- if (err)
+ if (err) {
+ dev_err(&idt82p33->client->dev,
+ "Failed at line %d in func %s!\n",
+ __LINE__,
+ __func__);
return err;
+ }
err = idt82p33_measure_tod_write_overhead(channel);
- if (err)
+ if (err) {
+ dev_err(&idt82p33->client->dev,
+ "Failed at line %d in func %s!\n",
+ __LINE__,
+ __func__);
return err;
+ }
err = _idt82p33_settime(channel, &ts);
@@ -728,6 +749,11 @@ static int idt82p33_adjfine(struct ptp_clock_info *ptp,
long scaled_ppm)
mutex_lock(&idt82p33->reg_lock);
err = _idt82p33_adjfine(channel, scaled_ppm);
+ if (err)
+ dev_err(&idt82p33->client->dev,
+ "Failed at line %d in func %s!\n",
+ __LINE__,
+ __func__);
mutex_unlock(&idt82p33->reg_lock);
return err;
@@ -751,10 +777,19 @@ static int idt82p33_adjtime(struct ptp_clock_info *ptp,
s64 delta_ns)
if (err) {
mutex_unlock(&idt82p33->reg_lock);
+ dev_err(&idt82p33->client->dev,
+ "Failed at line %d in func %s!\n",
+ __LINE__,
+ __func__);
return err;
}
err = idt82p33_sync_tod(channel, true);
+ if (err)
+ dev_err(&idt82p33->client->dev,
+ "Failed at line %d in func %s!\n",
+ __LINE__,
+ __func__);
mutex_unlock(&idt82p33->reg_lock);
@@ -770,6 +805,11 @@ static int idt82p33_gettime(struct ptp_clock_info *ptp,
struct timespec64 *ts)
mutex_lock(&idt82p33->reg_lock);
err = _idt82p33_gettime(channel, ts);
+ if (err)
+ dev_err(&idt82p33->client->dev,
+ "Failed at line %d in func %s!\n",
+ __LINE__,
+ __func__);
mutex_unlock(&idt82p33->reg_lock);
return err;
@@ -785,6 +825,11 @@ static int idt82p33_settime(struct ptp_clock_info *ptp,
mutex_lock(&idt82p33->reg_lock);
err = _idt82p33_settime(channel, ts);
+ if (err)
+ dev_err(&idt82p33->client->dev,
+ "Failed at
RE: [PATCH net 4/4] ptp: ptp_idt82p33: support individually configure output by index
Hi David/Richard
When you have time, can you take a look at this change? Thanks
Min
-Original Message-
From: min.li...@renesas.com
Sent: August 7, 2020 11:58 AM
To: richardcoch...@gmail.com
Cc: net...@vger.kernel.org; linux-kernel@vger.kernel.org; Min Li
Subject: [PATCH net 4/4] ptp: ptp_idt82p33: support individually configure
output by index
From: Min Li
Enable/disable individual output by index instead of by output_mask
Signed-off-by: Min Li
---
drivers/ptp/ptp_idt82p33.c | 62 ++
drivers/ptp/ptp_idt82p33.h | 2 ++
2 files changed, 48 insertions(+), 16 deletions(-)
diff --git a/drivers/ptp/ptp_idt82p33.c b/drivers/ptp/ptp_idt82p33.c index
2d62aed..b3c82d7 100644
--- a/drivers/ptp/ptp_idt82p33.c
+++ b/drivers/ptp/ptp_idt82p33.c
@@ -578,33 +578,46 @@ static long idt82p33_sync_tod_work_handler(struct
ptp_clock_info *ptp)
return ret;
}
-static int idt82p33_pps_enable(struct idt82p33_channel *channel, bool enable)
+static int idt82p33_output_enable(struct idt82p33_channel *channel,
+ bool enable, unsigned int outn)
{
struct idt82p33 *idt82p33 = channel->idt82p33;
- u8 mask, outn, val;
int err;
+ u8 val;
+
+ err = idt82p33_read(idt82p33, OUT_MUX_CNFG(outn), &val, sizeof(val));
+
+ if (err)
+ return err;
+
+ if (enable)
+ val &= ~SQUELCH_ENABLE;
+ else
+ val |= SQUELCH_ENABLE;
+
+ return idt82p33_write(idt82p33, OUT_MUX_CNFG(outn), &val,
+sizeof(val)); }
+
+static int idt82p33_output_mask_enable(struct idt82p33_channel *channel,
+ bool enable)
+{
+ u16 mask;
+ int err;
+ u8 outn;
mask = channel->output_mask;
outn = 0;
while (mask) {
- if (mask & 0x1) {
- err = idt82p33_read(idt82p33, OUT_MUX_CNFG(outn),
- &val, sizeof(val));
- if (err)
- return err;
- if (enable)
- val &= ~SQUELCH_ENABLE;
- else
- val |= SQUELCH_ENABLE;
+ if (mask & 0x1) {
- err = idt82p33_write(idt82p33, OUT_MUX_CNFG(outn),
-&val, sizeof(val));
+ err = idt82p33_output_enable(channel, enable, outn);
if (err)
return err;
}
+
mask >>= 0x1;
outn++;
}
@@ -612,6 +625,20 @@ static int idt82p33_pps_enable(struct idt82p33_channel
*channel, bool enable)
return 0;
}
+static int idt82p33_perout_enable(struct idt82p33_channel *channel,
+ bool enable,
+ struct ptp_perout_request *perout) {
+ unsigned int flags = perout->flags;
+
+ /* Enable/disable output based on output_mask */
+ if (flags == PEROUT_ENABLE_OUTPUT_MASK)
+ return idt82p33_output_mask_enable(channel, enable);
+
+ /* Enable/disable individual output instead */
+ return idt82p33_output_enable(channel, enable, perout->index); }
+
static int idt82p33_enable_tod(struct idt82p33_channel *channel) {
struct idt82p33 *idt82p33 = channel->idt82p33; @@ -625,7 +652,8 @@
static int idt82p33_enable_tod(struct idt82p33_channel *channel)
if (err)
return err;
- err = idt82p33_pps_enable(channel, false);
+ if (0)
+ err = idt82p33_output_mask_enable(channel, false);
if (err) {
dev_err(&idt82p33->client->dev,
@@ -681,14 +709,16 @@ static int idt82p33_enable(struct ptp_clock_info *ptp,
if (rq->type == PTP_CLK_REQ_PEROUT) {
if (!on)
- err = idt82p33_pps_enable(channel, false);
+ err = idt82p33_perout_enable(channel, false,
+&rq->perout);
/* Only accept a 1-PPS aligned to the second. */
else if (rq->perout.start.nsec || rq->perout.period.sec != 1 ||
rq->perout.period.nsec) {
err = -ERANGE;
} else
- err = idt82p33_pps_enable(channel, true);
+ err = idt82p33_perout_enable(channel, true,
+&rq->perout);
}
mutex_unlock(&idt82p33->reg_lock);
diff --git a/drivers/ptp/ptp_idt82p33.h b/drivers/ptp/ptp_idt82p33.h index
1dcd2c0..5008998 100644
--- a/drivers/ptp/ptp_idt82p33.h
+++ b/drivers/ptp/ptp_idt82p33.h
@@ -56,6 +56,8 @@
#define PLL_MODE_SHIFT
RE: [PATCH net 1/4] ptp: ptp_idt82p33: update to support adjphase
Hi David/Richard
When you have time, can you take a look at this change? Thanks
Min
-Original Message-
From: min.li...@renesas.com
Sent: August 7, 2020 11:56 AM
To: richardcoch...@gmail.com
Cc: net...@vger.kernel.org; linux-kernel@vger.kernel.org; Min Li
Subject: [PATCH net 1/4] ptp: ptp_idt82p33: update to support adjphase
From: Min Li
Add adjphase support for idt82p33xxx synchronization management unit.
Also fix n_per_out to the actual number of outputs.
Changes since v1:
- Break into small changes
Signed-off-by: Min Li
---
drivers/ptp/ptp_idt82p33.c | 48 +-
1 file changed, 47 insertions(+), 1 deletion(-)
diff --git a/drivers/ptp/ptp_idt82p33.c b/drivers/ptp/ptp_idt82p33.c index
179f6c4..bd1fbcd 100644
--- a/drivers/ptp/ptp_idt82p33.c
+++ b/drivers/ptp/ptp_idt82p33.c
@@ -674,6 +674,51 @@ static int idt82p33_enable(struct ptp_clock_info *ptp,
return err;
}
+static int idt82p33_adjwritephase(struct ptp_clock_info *ptp, s32
+offsetNs) {
+ struct idt82p33_channel *channel =
+ container_of(ptp, struct idt82p33_channel, caps);
+ struct idt82p33 *idt82p33 = channel->idt82p33;
+ s64 offsetInFs;
+ s64 offsetRegVal;
+ u8 val[4] = {0};
+ int err;
+
+ offsetInFs = (s64)(-offsetNs) * 100;
+
+ if (offsetInFs > WRITE_PHASE_OFFSET_LIMIT)
+ offsetInFs = WRITE_PHASE_OFFSET_LIMIT;
+ else if (offsetInFs < -WRITE_PHASE_OFFSET_LIMIT)
+ offsetInFs = -WRITE_PHASE_OFFSET_LIMIT;
+
+ /* Convert from phaseOffsetInFs to register value */
+ offsetRegVal = ((offsetInFs * 1000) / IDT_T0DPLL_PHASE_RESOL);
+
+ val[0] = offsetRegVal & 0xFF;
+ val[1] = (offsetRegVal >> 8) & 0xFF;
+ val[2] = (offsetRegVal >> 16) & 0xFF;
+ val[3] = (offsetRegVal >> 24) & 0x1F;
+ val[3] |= PH_OFFSET_EN;
+
+ mutex_lock(&idt82p33->reg_lock);
+
+ err = idt82p33_dpll_set_mode(channel, PLL_MODE_WPH);
+ if (err) {
+ dev_err(&idt82p33->client->dev,
+ "Failed at line %d in func %s!\n",
+ __LINE__,
+ __func__);
+ goto out;
+ }
+
+ err = idt82p33_write(idt82p33, channel->dpll_phase_cnfg, val,
+sizeof(val));
+
+out:
+ mutex_unlock(&idt82p33->reg_lock);
+ return err;
+}
+
static int idt82p33_adjfine(struct ptp_clock_info *ptp, long scaled_ppm) {
struct idt82p33_channel *channel =
@@ -784,6 +829,8 @@ static void idt82p33_caps_init(struct ptp_clock_info *caps)
{
caps->owner = THIS_MODULE;
caps->max_adj = 92000;
+ caps->n_per_out = 11;
+ caps->adjphase = idt82p33_adjwritephase;
caps->adjfine = idt82p33_adjfine;
caps->adjtime = idt82p33_adjtime;
caps->gettime64 = idt82p33_gettime;
@@ -810,7 +857,6 @@ static int idt82p33_enable_channel(struct idt82p33
*idt82p33, u32 index)
idt82p33_caps_init(&channel->caps);
snprintf(channel->caps.name, sizeof(channel->caps.name),
"IDT 82P33 PLL%u", index);
- channel->caps.n_per_out = hweight8(channel->output_mask);
err = idt82p33_dpll_set_mode(channel, PLL_MODE_DCO);
if (err)
--
2.7.4
