Re: [PATCH 2/2] ptp: Add a ptp clock driver for IDT ClockMatrix.
Hi Andrew,
On Wed, Sep 18, 2019 at 05:18:03PM EDT, Andrew Lunn wrote:
>On Wed, Sep 18, 2019 at 04:06:38PM -0400, vincent.cheng...@renesas.com wrote:
>
>> +static s32 idtcm_xfer(struct idtcm *idtcm,
>> + u8 regaddr,
>> + u8 *buf,
>> + u16 count,
>> + bool write)
>> +{
>> +struct i2c_client *client = idtcm->client;
>> +struct i2c_msg msg[2];
>> +s32 cnt;
>> +
>> +msg[0].addr = client->addr;
>> +msg[0].flags = 0;
>> +msg[0].len = 1;
>> +msg[0].buf = ®addr;
>> +
>> +msg[1].addr = client->addr;
>> +msg[1].flags = write ? 0 : I2C_M_RD;
>> +msg[1].len = count;
>> +msg[1].buf = buf;
>> +
>> +cnt = i2c_transfer(client->adapter, msg, 2);
>> +
>> +if (cnt < 0) {
>> +pr_err("i2c_transfer returned %d\n", cnt);
>
>dev_err(client->dev, "i2c_transfer returned %d\n", cnt);
>
>We then have an idea which device has a transfer error.
>
>Please try to not use pr_err() when you have some sort of device.
Sure thing, will replace pr_err() with dev_err().
>> +static s32 idtcm_state_machine_reset(struct idtcm *idtcm)
>> +{
>> +s32 err;
>> +u8 byte = SM_RESET_CMD;
>> +
>> +err = idtcm_write(idtcm, RESET_CTRL, SM_RESET, &byte, sizeof(byte));
>> +
>> +if (!err) {
>> +/* delay */
>> +set_current_state(TASK_INTERRUPTIBLE);
>> +schedule_timeout(_msecs_to_jiffies(POST_SM_RESET_DELAY_MS));
>
>Maybe use msleep_interruptable()?
Yes, will try using msleep_interruptable() and will replace if it works.
>> +static s32 idtcm_load_firmware(struct idtcm *idtcm,
>> + struct device *dev)
>> +{
>> +const struct firmware *fw;
>> +struct idtcm_fwrc *rec;
>> +u32 regaddr;
>> +s32 err;
>> +s32 len;
>> +u8 val;
>> +u8 loaddr;
>> +
>> +pr_info("requesting firmware '%s'\n", FW_FILENAME);
>
>dev_debug()
Thanks, will make the change.
>> +
>> +err = request_firmware(&fw, FW_FILENAME, dev);
>> +
>> +if (err)
>> +return err;
>> +
>> +pr_info("firmware size %zu bytes\n", fw->size);
>
>dev_debug()
>
>Maybe look through all your pr_info and downgrade most of them to
>dev_debug()
Yes, will go through and downgrade to dev_debug() accordingly.
Thanks,
Vincent
Re: [PATCH 2/2] ptp: Add a ptp clock driver for IDT ClockMatrix.
Hi, Thank you for the patch! Yet something to improve: [auto build test ERROR on linus/master] [cannot apply to v5.3 next-20190918] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system. BTW, we also suggest to use '--base' option to specify the base tree in git format-patch, please see https://stackoverflow.com/a/37406982] url: https://github.com/0day-ci/linux/commits/vincent-cheng-xh-renesas-com/dt-bindings-ptp-Add-binding-doc-for-IDT-ClockMatrix-based-PTP-clock/20190919-043257 config: i386-allmodconfig (attached as .config) compiler: gcc-7 (Debian 7.4.0-13) 7.4.0 reproduce: # save the attached .config to linux build tree make ARCH=i386 If you fix the issue, kindly add following tag Reported-by: kbuild test robot All errors (new ones prefixed by >>): >> ERROR: "__divdi3" [drivers/ptp/ptp_clockmatrix.ko] undefined! --- 0-DAY kernel test infrastructureOpen Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation .config.gz Description: application/gzip
Re: [PATCH 2/2] ptp: Add a ptp clock driver for IDT ClockMatrix.
On Wed, Sep 18, 2019 at 04:06:38PM -0400, vincent.cheng...@renesas.com wrote:
> From: Vincent Cheng
>
> The IDT ClockMatrix (TM) family includes integrated devices that provide
> eight PLL channels. Each PLL channel can be independently configured as a
> frequency synthesizer, jitter attenuator, digitally controlled
> oscillator (DCO), or a digital phase lock loop (DPLL). Typically
> these devices are used as timing references and clock sources for PTP
> applications. This patch adds support for the device.
>
> Co-developed-by: Richard Cochran
> Signed-off-by: Richard Cochran
> Signed-off-by: Vincent Cheng
Hi Vincent
> +static s32 idtcm_xfer(struct idtcm *idtcm,
> + u8 regaddr,
> + u8 *buf,
> + u16 count,
> + bool write)
> +{
> + struct i2c_client *client = idtcm->client;
> + struct i2c_msg msg[2];
> + s32 cnt;
> +
> + msg[0].addr = client->addr;
> + msg[0].flags = 0;
> + msg[0].len = 1;
> + msg[0].buf = ®addr;
> +
> + msg[1].addr = client->addr;
> + msg[1].flags = write ? 0 : I2C_M_RD;
> + msg[1].len = count;
> + msg[1].buf = buf;
> +
> + cnt = i2c_transfer(client->adapter, msg, 2);
> +
> + if (cnt < 0) {
> + pr_err("i2c_transfer returned %d\n", cnt);
dev_err(client->dev, "i2c_transfer returned %d\n", cnt);
We then have an idea which device has a transfer error.
Please try to not use pr_err() when you have some sort of device.
> +static s32 idtcm_state_machine_reset(struct idtcm *idtcm)
> +{
> + s32 err;
> + u8 byte = SM_RESET_CMD;
> +
> + err = idtcm_write(idtcm, RESET_CTRL, SM_RESET, &byte, sizeof(byte));
> +
> + if (!err) {
> + /* delay */
> + set_current_state(TASK_INTERRUPTIBLE);
> + schedule_timeout(_msecs_to_jiffies(POST_SM_RESET_DELAY_MS));
Maybe use msleep_interruptable()?
> + }
> +
> + return err;
> +}
> +
> +static s32 idtcm_load_firmware(struct idtcm *idtcm,
> +struct device *dev)
> +{
> + const struct firmware *fw;
> + struct idtcm_fwrc *rec;
> + u32 regaddr;
> + s32 err;
> + s32 len;
> + u8 val;
> + u8 loaddr;
> +
> + pr_info("requesting firmware '%s'\n", FW_FILENAME);
dev_debug()
> +
> + err = request_firmware(&fw, FW_FILENAME, dev);
> +
> + if (err)
> + return err;
> +
> + pr_info("firmware size %zu bytes\n", fw->size);
dev_debug()
Maybe look through all your pr_info and downgrade most of them to
dev_debug()
Andrew
[PATCH 2/2] ptp: Add a ptp clock driver for IDT ClockMatrix.
From: Vincent Cheng The IDT ClockMatrix (TM) family includes integrated devices that provide eight PLL channels. Each PLL channel can be independently configured as a frequency synthesizer, jitter attenuator, digitally controlled oscillator (DCO), or a digital phase lock loop (DPLL). Typically these devices are used as timing references and clock sources for PTP applications. This patch adds support for the device. Co-developed-by: Richard Cochran Signed-off-by: Richard Cochran Signed-off-by: Vincent Cheng --- drivers/ptp/Kconfig | 12 + drivers/ptp/Makefile |1 + drivers/ptp/idt8a340_reg.h| 659 drivers/ptp/ptp_clockmatrix.c | 1384 + drivers/ptp/ptp_clockmatrix.h | 123 5 files changed, 2179 insertions(+) create mode 100644 drivers/ptp/idt8a340_reg.h create mode 100644 drivers/ptp/ptp_clockmatrix.c create mode 100644 drivers/ptp/ptp_clockmatrix.h diff --git a/drivers/ptp/Kconfig b/drivers/ptp/Kconfig index 960961f..16c7c90 100644 --- a/drivers/ptp/Kconfig +++ b/drivers/ptp/Kconfig @@ -119,4 +119,16 @@ config PTP_1588_CLOCK_KVM To compile this driver as a module, choose M here: the module will be called ptp_kvm. +config PTP_1588_CLOCK_IDTCM + tristate "IDT CLOCKMATRIX as PTP clock" + select PTP_1588_CLOCK + default n + help + This driver adds support for using IDT CLOCKMATRIX(TM) as a PTP + clock. This clock is only useful if your time stamping MAC + is connected to the IDT chip. + + To compile this driver as a module, choose M here: the module + will be called ptp_clockmatrix. + endmenu diff --git a/drivers/ptp/Makefile b/drivers/ptp/Makefile index 677d1d1..9791ba9 100644 --- a/drivers/ptp/Makefile +++ b/drivers/ptp/Makefile @@ -12,3 +12,4 @@ obj-$(CONFIG_PTP_1588_CLOCK_KVM) += ptp_kvm.o obj-$(CONFIG_PTP_1588_CLOCK_QORIQ) += ptp-qoriq.o ptp-qoriq-y+= ptp_qoriq.o ptp-qoriq-$(CONFIG_DEBUG_FS) += ptp_qoriq_debugfs.o +obj-$(CONFIG_PTP_1588_CLOCK_IDTCM) += ptp_clockmatrix.o diff --git a/drivers/ptp/idt8a340_reg.h b/drivers/ptp/idt8a340_reg.h new file mode 100644 index 000..9263bc3 --- /dev/null +++ b/drivers/ptp/idt8a340_reg.h @@ -0,0 +1,659 @@ +/* SPDX-License-Identifier: GPL-2.0+ */ +/* idt8a340_reg.h + * + * Originally generated by regen.tcl on Thu Feb 14 19:23:44 PST 2019 + * https://github.com/richardcochran/regen + * + * Hand modified to include some HW registers. + * Based on 4.8.0, SCSR rev C commit a03c7ae5 + */ +#ifndef HAVE_IDT8A340_REG +#define HAVE_IDT8A340_REG + +#define PAGE_ADDR_BASE0x +#define PAGE_ADDR 0x00fc + +#define HW_REVISION 0x8180 +#define REV_ID0x007a + +#define HW_DPLL_0 (0x8a00) +#define HW_DPLL_1 (0x8b00) +#define HW_DPLL_2 (0x8c00) +#define HW_DPLL_3 (0x8d00) + +#define HW_DPLL_TOD_SW_TRIG_ADDR__0 (0x080) +#define HW_DPLL_TOD_CTRL_1(0x089) +#define HW_DPLL_TOD_CTRL_2(0x08A) +#define HW_DPLL_TOD_OVR__0(0x098) +#define HW_DPLL_TOD_OUT_0__0 (0x0B0) + +#define HW_Q0_Q1_CH_SYNC_CTRL_0 (0xa740) +#define HW_Q0_Q1_CH_SYNC_CTRL_1 (0xa741) +#define HW_Q2_Q3_CH_SYNC_CTRL_0 (0xa742) +#define HW_Q2_Q3_CH_SYNC_CTRL_1 (0xa743) +#define HW_Q4_Q5_CH_SYNC_CTRL_0 (0xa744) +#define HW_Q4_Q5_CH_SYNC_CTRL_1 (0xa745) +#define HW_Q6_Q7_CH_SYNC_CTRL_0 (0xa746) +#define HW_Q6_Q7_CH_SYNC_CTRL_1 (0xa747) +#define HW_Q8_CH_SYNC_CTRL_0 (0xa748) +#define HW_Q8_CH_SYNC_CTRL_1 (0xa749) +#define HW_Q9_CH_SYNC_CTRL_0 (0xa74a) +#define HW_Q9_CH_SYNC_CTRL_1 (0xa74b) +#define HW_Q10_CH_SYNC_CTRL_0 (0xa74c) +#define HW_Q10_CH_SYNC_CTRL_1 (0xa74d) +#define HW_Q11_CH_SYNC_CTRL_0 (0xa74e) +#define HW_Q11_CH_SYNC_CTRL_1 (0xa74f) + +#define SYNC_SOURCE_DPLL0_TOD_PPS 0x14 +#define SYNC_SOURCE_DPLL1_TOD_PPS 0x15 +#define SYNC_SOURCE_DPLL2_TOD_PPS 0x16 +#define SYNC_SOURCE_DPLL3_TOD_PPS 0x17 + +#define SYNCTRL1_MASTER_SYNC_RST BIT(7) +#define SYNCTRL1_MASTER_SYNC_TRIG BIT(5) +#define SYNCTRL1_TOD_SYNC_TRIG BIT(4) +#define SYNCTRL1_FBDIV_FRAME_SYNC_TRIG BIT(3) +#define SYNCTRL1_FBDIV_SYNC_TRIG BIT(2) +#define SYNCTRL1_Q1_DIV_SYNC_TRIG BIT(1) +#define SYNCTRL1_Q0_DIV_SYNC_TRIG BIT(0) + +#define RESET_CTRL0xc000 +#define SM_RESET 0x0012 +#define SM_RESET_CMD 0x5A + +#define GENERAL_STATUS0xc014 +#define HW_REV_ID 0x000A +#define BOND_ID 0x000B +#define
