From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755080Ab2CJLCt (ORCPT ); Sat, 10 Mar 2012 06:02:49 -0500 Received: from mail-wi0-f172.google.com ([209.85.212.172]:51570 "EHLO mail-wi0-f172.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754843Ab2CJLCq (ORCPT ); Sat, 10 Mar 2012 06:02:46 -0500 Date: Sat, 10 Mar 2012 12:02:32 +0100 From: Richard Cochran To: Takahiro Shimizu Cc: jeffrey.t.kirsher@intel.com, davem@davemloft.net, lucas.demarchi@profusion.mobi, mirq-linux@rere.qmqm.pl, paul.gortmaker@windriver.com, jdmason@kudzu.us, john.stultz@linaro.org, arnd@arndb.de, khc@pm.waw.pl, netdev@vger.kernel.org, linux-kernel@vger.kernel.org, qi.wang@intel.com, yong.y.wang@intel.com, joel.clark@intel.com, kok.howg.ewe@intel.com Subject: Re: [PATCH RE-SUBMIT 1/2] supports eg20t ptp clock Message-ID: <20120310110230.GB3489@netboy.at.omicron.at> References: <1331194587-29114-1-git-send-email-tshimizu818@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1331194587-29114-1-git-send-email-tshimizu818@gmail.com> User-Agent: Mutt/1.5.20 (2009-06-14) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org This looks good, for the most part. However, I did find a few issues though, below. Thanks, Richard On Thu, Mar 08, 2012 at 05:16:26PM +0900, Takahiro Shimizu wrote: > Supports EG20T ptp clock in the driver > > Changes e-mail address. > > Adds number. > > Signed-off-by: Takahiro Shimizu > --- > drivers/ptp/Kconfig | 13 + > drivers/ptp/Makefile | 1 + > drivers/ptp/ptp_pch.c | 731 +++++++++++++++++++++++++++++++++++++++++++++++++ > 3 files changed, 745 insertions(+), 0 deletions(-) > create mode 100644 drivers/ptp/ptp_pch.c > > diff --git a/drivers/ptp/Kconfig b/drivers/ptp/Kconfig > index 68d7201..cd9bc3b 100644 > --- a/drivers/ptp/Kconfig > +++ b/drivers/ptp/Kconfig > @@ -72,4 +72,17 @@ config DP83640_PHY > In order for this to work, your MAC driver must also > implement the skb_tx_timetamp() function. > > +config PTP_1588_CLOCK_PCH > + tristate "Intel PCH EG20T as PTP clock" > + depends on PTP_1588_CLOCK > + depends on PCH_GBE > + help > + This driver adds support for using the PCH EG20T as a PTP > + clock. This clock is only useful if your PTP programs are > + getting hardware time stamps on the PTP Ethernet packets > + using the SO_TIMESTAMPING API. > + > + To compile this driver as a module, choose M here: the module > + will be called ptp_pch. > + > endmenu > diff --git a/drivers/ptp/Makefile b/drivers/ptp/Makefile > index f6933e8..8b58597 100644 > --- a/drivers/ptp/Makefile > +++ b/drivers/ptp/Makefile > @@ -5,3 +5,4 @@ > ptp-y := ptp_clock.o ptp_chardev.o ptp_sysfs.o > obj-$(CONFIG_PTP_1588_CLOCK) += ptp.o > obj-$(CONFIG_PTP_1588_CLOCK_IXP46X) += ptp_ixp46x.o > +obj-$(CONFIG_PTP_1588_CLOCK_PCH) += ptp_pch.o > diff --git a/drivers/ptp/ptp_pch.c b/drivers/ptp/ptp_pch.c > new file mode 100644 > index 0000000..1a06e02 > --- /dev/null > +++ b/drivers/ptp/ptp_pch.c > @@ -0,0 +1,731 @@ > +/* > + * PTP 1588 clock using the EG20T PCH > + * > + * Copyright (C) 2010 OMICRON electronics GmbH > + * Copyright (C) 2011-2012 LAPIS SEMICONDUCTOR Co., LTD. > + * > + * This code was derived from the IXP46X driver. > + * > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU General Public License as published by > + * the Free Software Foundation; version 2 of the License. > + * > + * This program is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > + * GNU General Public License for more details. > + * > + * You should have received a copy of the GNU General Public License > + * along with this program; if not, write to the Free Software > + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307, USA. > + */ > + > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > + > +#define STATION_ADDR_LEN 20 > +#define PCI_DEVICE_ID_PCH_1588 0x8819 > +#define IO_MEM_BAR 1 > + > +#define DEFAULT_ADDEND 0xA0000000 The frequency adjustment problems that you reported on linuxptp-users would be caused by wrong values of DEFAULT_ADDEND and TICKS_NS_SHIFT. What is the input clock frequency for this hardware? > +#define TICKS_NS_SHIFT 5 You have defined this TICKS_NS_SHIFT twice, once here, and once in pch_gbe_main.c in the following patch. You don't need to use this macro in pch_gbe_main.c at all. See below. [ Even if you did need this macro in two different .c files, then the right way would be to use a header file. BTW the PHC driver does not necessarily have to go into drivers/ptp. If it works better to keep the PHC driver in the same directory as the MAC driver, to use a shared header for example, then that is fine, too. See gianfar for an example of that. ] > +#define N_EXT_TS 2 > + > +enum pch_status { > + PCH_SUCCESS, > + PCH_INVALIDPARAM, > + PCH_NOTIMESTAMP, > + PCH_INTERRUPTMODEINUSE, > + PCH_FAILED, > + PCH_UNSUPPORTED, > +}; > +/** > + * struct pch_ts_regs - IEEE 1588 registers > + */ > +struct pch_ts_regs { > + u32 control; > + u32 event; > + u32 addend; > + u32 accum; > + u32 test; > + u32 ts_compare; > + u32 rsystime_lo; > + u32 rsystime_hi; > + u32 systime_lo; > + u32 systime_hi; > + u32 trgt_lo; > + u32 trgt_hi; > + u32 asms_lo; > + u32 asms_hi; > + u32 amms_lo; > + u32 amms_hi; > + u32 ch_control; > + u32 ch_event; > + u32 tx_snap_lo; > + u32 tx_snap_hi; > + u32 rx_snap_lo; > + u32 rx_snap_hi; > + u32 src_uuid_lo; > + u32 src_uuid_hi; > + u32 can_status; > + u32 can_snap_lo; > + u32 can_snap_hi; > + u32 ts_sel; > + u32 ts_st[6]; > + u32 reserve1[14]; > + u32 stl_max_set_en; > + u32 stl_max_set; > + u32 reserve2[13]; > + u32 srst; > +}; > + > +#define PCH_TSC_RESET (1 << 0) > +#define PCH_TSC_TTM_MASK (1 << 1) > +#define PCH_TSC_ASMS_MASK (1 << 2) > +#define PCH_TSC_AMMS_MASK (1 << 3) > +#define PCH_TSC_PPSM_MASK (1 << 4) > +#define PCH_TSE_TTIPEND (1 << 1) > +#define PCH_TSE_SNS (1 << 2) > +#define PCH_TSE_SNM (1 << 3) > +#define PCH_TSE_PPS (1 << 4) > +#define PCH_CC_MM (1 << 0) > +#define PCH_CC_TA (1 << 1) > + > +#define PCH_CC_MODE_SHIFT 16 > +#define PCH_CC_MODE_MASK 0x001F0000 > +#define PCH_CC_VERSION (1 << 31) > +#define PCH_CE_TXS (1 << 0) > +#define PCH_CE_RXS (1 << 1) > +#define PCH_CE_OVR (1 << 0) > +#define PCH_CE_VAL (1 << 1) > +#define PCH_ECS_ETH (1 << 0) > + > +#define PCH_ECS_CAN (1 << 1) > +#define PCH_STATION_BYTES 6 > + > +#define PCH_IEEE1588_ETH (1 << 0) > +#define PCH_IEEE1588_CAN (1 << 1) > +/** > + * struct pch_dev - Driver private data > + */ > +struct pch_dev { > + struct pch_ts_regs *regs; > + struct ptp_clock *ptp_clock; > + struct ptp_clock_info caps; > + int exts0_enabled; > + int exts1_enabled; > + > + u32 mem_base; > + u32 mem_size; > + u32 irq; > + struct pci_dev *pdev; > + spinlock_t register_lock; > +}; > + > +/** > + * struct pch_params - 1588 module parameter > + */ > +struct pch_params { > + u8 station[STATION_ADDR_LEN]; > +}; > + > +/* structure to hold the module parameters */ > +static struct pch_params pch_param = { > + "00:00:00:00:00:00" > +}; > + > +/* > + * Register access functions > + */ > +static inline void pch_eth_enable_set(struct pch_dev *chip) > +{ > + u32 val; > + /* SET the eth_enable bit */ > + val = ioread32(&chip->regs->ts_sel) | (PCH_ECS_ETH); > + iowrite32(val, (&chip->regs->ts_sel)); > +} > + > +static u64 pch_systime_read(struct pch_ts_regs *regs) > +{ > + u64 ns; > + u32 lo, hi; > + > + lo = ioread32(®s->systime_lo); > + hi = ioread32(®s->systime_hi); > + > + ns = ((u64) hi) << 32; > + ns |= lo; > + ns <<= TICKS_NS_SHIFT; > + > + return ns; > +} > + > +static void pch_systime_write(struct pch_ts_regs *regs, u64 ns) > +{ > + u32 hi, lo; > + > + ns >>= TICKS_NS_SHIFT; > + hi = ns >> 32; > + lo = ns & 0xffffffff; > + > + iowrite32(lo, ®s->systime_lo); > + iowrite32(hi, ®s->systime_hi); > +} > + > +static inline void pch_block_reset(struct pch_dev *chip) > +{ > + u32 val; > + /* Reset Hardware Assist block */ > + val = ioread32(&chip->regs->control) | PCH_TSC_RESET; > + iowrite32(val, (&chip->regs->control)); > + val = val & ~PCH_TSC_RESET; > + iowrite32(val, (&chip->regs->control)); > +} > + > +u32 pch_ch_control_read(struct pci_dev *pdev) > +{ > + struct pch_dev *chip = pci_get_drvdata(pdev); > + u32 val; > + > + val = ioread32(&chip->regs->ch_control); > + > + return val; > +} > +EXPORT_SYMBOL(pch_ch_control_read); > + > +void pch_ch_control_write(struct pci_dev *pdev, u32 val) > +{ > + struct pch_dev *chip = pci_get_drvdata(pdev); > + > + iowrite32(val, (&chip->regs->ch_control)); > +} > +EXPORT_SYMBOL(pch_ch_control_write); > + > +u32 pch_ch_event_read(struct pci_dev *pdev) > +{ > + struct pch_dev *chip = pci_get_drvdata(pdev); > + u32 val; > + > + val = ioread32(&chip->regs->ch_event); > + > + return val; > +} > +EXPORT_SYMBOL(pch_ch_event_read); > + > +void pch_ch_event_write(struct pci_dev *pdev, u32 val) > +{ > + struct pch_dev *chip = pci_get_drvdata(pdev); > + > + iowrite32(val, (&chip->regs->ch_event)); > +} > +EXPORT_SYMBOL(pch_ch_event_write); > + > +u32 pch_src_uuid_lo_read(struct pci_dev *pdev) > +{ > + struct pch_dev *chip = pci_get_drvdata(pdev); > + u32 val; > + > + val = ioread32(&chip->regs->src_uuid_lo); > + > + return val; > +} > +EXPORT_SYMBOL(pch_src_uuid_lo_read); > + > +u32 pch_src_uuid_hi_read(struct pci_dev *pdev) > +{ > + struct pch_dev *chip = pci_get_drvdata(pdev); > + u32 val; > + > + val = ioread32(&chip->regs->src_uuid_hi); > + > + return val; > +} > +EXPORT_SYMBOL(pch_src_uuid_hi_read); > + > +u64 pch_rx_snap_read(struct pci_dev *pdev) > +{ > + struct pch_dev *chip = pci_get_drvdata(pdev); > + u64 ns; > + u32 lo, hi; > + > + lo = ioread32(&chip->regs->rx_snap_lo); > + hi = ioread32(&chip->regs->rx_snap_hi); > + > + ns = ((u64) hi) << 32; > + ns |= lo; Here you should correct the 'ns' using the TICKS_NS_SHIFT so that the return value is in nanoseconds. > + > + return ns; > +} > +EXPORT_SYMBOL(pch_rx_snap_read); > + > +u64 pch_tx_snap_read(struct pci_dev *pdev) > +{ > + struct pch_dev *chip = pci_get_drvdata(pdev); > + u64 ns; > + u32 lo, hi; > + > + lo = ioread32(&chip->regs->tx_snap_lo); > + hi = ioread32(&chip->regs->tx_snap_hi); > + > + ns = ((u64) hi) << 32; > + ns |= lo; Here again, make the return value be in nanoseconds. Then you can remove TICKS_NS_SHIFT from pch_gbe_main.c altogether. > + > + return ns; > +} > +EXPORT_SYMBOL(pch_tx_snap_read); > + > +/* This function enables all 64 bits in system time registers [high & low]. > +This is a work-around for non continuous value in the SystemTime Register*/ > +static void pch_set_system_time_count(struct pch_dev *chip) > +{ > + iowrite32(0x01, &chip->regs->stl_max_set_en); > + iowrite32(0xFFFFFFFF, &chip->regs->stl_max_set); > + iowrite32(0x00, &chip->regs->stl_max_set_en); > +} > + > +static void pch_reset(struct pch_dev *chip) > +{ > + /* Reset Hardware Assist */ > + pch_block_reset(chip); > + > + /* enable all 32 bits in system time registers */ > + pch_set_system_time_count(chip); > +} > + > +/** > + * pch_set_station_address() - This API sets the station address used by > + * IEEE 1588 hardware when looking at PTP > + * traffic on the ethernet interface > + * @addr: dress which contain the column separated address to be used. > + */ > +static int pch_set_station_address(u8 *addr, struct pci_dev *pdev) > +{ > + s32 i; > + struct pch_dev *chip = pci_get_drvdata(pdev); > + > + /* Verify the parameter */ > + if ((chip->regs == 0) || addr == (u8 *)NULL) { > + dev_err(&pdev->dev, > + "invalid params returning PCH_INVALIDPARAM\n"); > + return PCH_INVALIDPARAM; > + } > + /* For all station address bytes */ > + for (i = 0; i < PCH_STATION_BYTES; i++) { > + u32 val; > + s32 tmp; > + > + tmp = hex_to_bin(addr[i * 3]); > + if (tmp < 0) { > + dev_err(&pdev->dev, > + "invalid params returning PCH_INVALIDPARAM\n"); > + return PCH_INVALIDPARAM; > + } > + val = tmp * 16; > + tmp = hex_to_bin(addr[(i * 3) + 1]); > + if (tmp < 0) { > + dev_err(&pdev->dev, > + "invalid params returning PCH_INVALIDPARAM\n"); > + return PCH_INVALIDPARAM; > + } > + val += tmp; > + /* Expects ':' separated addresses */ > + if ((i < 5) && (addr[(i * 3) + 2] != ':')) { > + dev_err(&pdev->dev, > + "invalid params returning PCH_INVALIDPARAM\n"); > + return PCH_INVALIDPARAM; > + } > + > + /* Ideally we should set the address only after validating > + entire string */ > + dev_dbg(&pdev->dev, "invoking pch_station_set\n"); > + iowrite32(val, &chip->regs->ts_st[i]); > + } > + return 0; > +} > + > +/* > + * Interrupt service routine > + */ > +static irqreturn_t isr(int irq, void *priv) > +{ > + struct pch_dev *pch_dev = priv; > + struct pch_ts_regs *regs = pch_dev->regs; > + struct ptp_clock_event event; > + u32 ack = 0, lo, hi, val; > + > + val = ioread32(®s->event); > + > + if (val & PCH_TSE_SNS) { > + ack |= PCH_TSE_SNS; > + if (pch_dev->exts0_enabled) { > + hi = ioread32(®s->asms_hi); > + lo = ioread32(®s->asms_lo); > + event.type = PTP_CLOCK_EXTTS; > + event.index = 0; > + event.timestamp = ((u64) hi) << 32; > + event.timestamp |= lo; > + event.timestamp <<= TICKS_NS_SHIFT; > + ptp_clock_event(pch_dev->ptp_clock, &event); > + } > + } > + > + if (val & PCH_TSE_SNM) { > + ack |= PCH_TSE_SNM; > + if (pch_dev->exts1_enabled) { > + hi = ioread32(®s->amms_hi); > + lo = ioread32(®s->amms_lo); > + event.type = PTP_CLOCK_EXTTS; > + event.index = 1; > + event.timestamp = ((u64) hi) << 32; > + event.timestamp |= lo; > + event.timestamp <<= TICKS_NS_SHIFT; > + ptp_clock_event(pch_dev->ptp_clock, &event); > + } > + } > + > + if (val & PCH_TSE_TTIPEND) > + ack |= PCH_TSE_TTIPEND; /* this bit seems to be always set */ > + > + if (ack) { > + iowrite32(ack, ®s->event); > + return IRQ_HANDLED; > + } else > + return IRQ_NONE; > +} > + > +/* > + * PTP clock operations > + */ > + > +static int ptp_pch_adjfreq(struct ptp_clock_info *ptp, s32 ppb) > +{ > + u64 adj; > + u32 diff, addend; > + int neg_adj = 0; > + struct pch_dev *pch_dev = container_of(ptp, struct pch_dev, caps); > + struct pch_ts_regs *regs = pch_dev->regs; > + > + if (ppb < 0) { > + neg_adj = 1; > + ppb = -ppb; > + } > + addend = DEFAULT_ADDEND; > + adj = addend; > + adj *= ppb; > + diff = div_u64(adj, 1000000000ULL); > + > + addend = neg_adj ? addend - diff : addend + diff; > + > + iowrite32(addend, ®s->addend); > + > + return 0; > +} > + > +static int ptp_pch_adjtime(struct ptp_clock_info *ptp, s64 delta) > +{ > + s64 now; > + unsigned long flags; > + struct pch_dev *pch_dev = container_of(ptp, struct pch_dev, caps); > + struct pch_ts_regs *regs = pch_dev->regs; > + > + spin_lock_irqsave(&pch_dev->register_lock, flags); > + now = pch_systime_read(regs); > + now += delta; > + pch_systime_write(regs, now); > + spin_unlock_irqrestore(&pch_dev->register_lock, flags); > + > + return 0; > +} > + > +static int ptp_pch_gettime(struct ptp_clock_info *ptp, struct timespec *ts) > +{ > + u64 ns; > + u32 remainder; > + unsigned long flags; > + struct pch_dev *pch_dev = container_of(ptp, struct pch_dev, caps); > + struct pch_ts_regs *regs = pch_dev->regs; > + > + spin_lock_irqsave(&pch_dev->register_lock, flags); > + ns = pch_systime_read(regs); > + spin_unlock_irqrestore(&pch_dev->register_lock, flags); > + > + ts->tv_sec = div_u64_rem(ns, 1000000000, &remainder); > + ts->tv_nsec = remainder; > + return 0; > +} > + > +static int ptp_pch_settime(struct ptp_clock_info *ptp, > + const struct timespec *ts) > +{ > + u64 ns; > + unsigned long flags; > + struct pch_dev *pch_dev = container_of(ptp, struct pch_dev, caps); > + struct pch_ts_regs *regs = pch_dev->regs; > + > + ns = ts->tv_sec * 1000000000ULL; > + ns += ts->tv_nsec; > + > + spin_lock_irqsave(&pch_dev->register_lock, flags); > + pch_systime_write(regs, ns); > + spin_unlock_irqrestore(&pch_dev->register_lock, flags); > + > + return 0; > +} > + > +static int ptp_pch_enable(struct ptp_clock_info *ptp, > + struct ptp_clock_request *rq, int on) > +{ > + struct pch_dev *pch_dev = container_of(ptp, struct pch_dev, caps); > + > + switch (rq->type) { > + case PTP_CLK_REQ_EXTTS: > + switch (rq->extts.index) { > + case 0: > + pch_dev->exts0_enabled = on ? 1 : 0; > + break; > + case 1: > + pch_dev->exts1_enabled = on ? 1 : 0; > + break; > + default: > + return -EINVAL; > + } > + return 0; > + default: > + break; > + } > + > + return -EOPNOTSUPP; > +} > + > +static struct ptp_clock_info ptp_pch_caps = { > + .owner = THIS_MODULE, > + .name = "PCH timer", > + .max_adj = 50000000, > + .n_ext_ts = N_EXT_TS, > + .pps = 0, > + .adjfreq = ptp_pch_adjfreq, > + .adjtime = ptp_pch_adjtime, > + .gettime = ptp_pch_gettime, > + .settime = ptp_pch_settime, > + .enable = ptp_pch_enable, > +}; > + > + > +#ifdef CONFIG_PM > +static s32 pch_suspend(struct pci_dev *pdev, pm_message_t state) > +{ > + pci_disable_device(pdev); > + pci_enable_wake(pdev, PCI_D3hot, 0); > + > + if (pci_save_state(pdev) != 0) { > + dev_err(&pdev->dev, "could not save PCI config state\n"); > + return -ENOMEM; > + } > + pci_set_power_state(pdev, pci_choose_state(pdev, state)); > + > + return 0; > +} > + > +static s32 pch_resume(struct pci_dev *pdev) > +{ > + s32 ret; > + > + pci_set_power_state(pdev, PCI_D0); > + pci_restore_state(pdev); > + ret = pci_enable_device(pdev); > + if (ret) { > + dev_err(&pdev->dev, "pci_enable_device failed\n"); > + return ret; > + } > + pci_enable_wake(pdev, PCI_D3hot, 0); > + return 0; > +} > +#else > +#define pch_suspend NULL > +#define pch_resume NULL > +#endif > + > +static void __devexit pch_remove(struct pci_dev *pdev) > +{ > + struct pch_dev *chip = pci_get_drvdata(pdev); > + > + ptp_clock_unregister(chip->ptp_clock); > + /* free the interrupt */ > + if (pdev->irq != 0) > + free_irq(pdev->irq, chip); > + > + /* unmap the virtual IO memory space */ > + if (chip->regs != 0) { > + iounmap(chip->regs); > + chip->regs = 0; > + } > + /* release the reserved IO memory space */ > + if (chip->mem_base != 0) { > + release_mem_region(chip->mem_base, chip->mem_size); > + chip->mem_base = 0; > + } > + pci_disable_device(pdev); > + kfree(chip); > + dev_info(&pdev->dev, "complete\n"); > +} > + > +static s32 __devinit > +pch_probe(struct pci_dev *pdev, const struct pci_device_id *id) > +{ > + s32 ret; > + unsigned long flags; > + struct pch_dev *chip; > + > + chip = kzalloc(sizeof(struct pch_dev), GFP_KERNEL); > + if (chip == NULL) > + return -ENOMEM; > + > + /* enable the 1588 pci device */ > + ret = pci_enable_device(pdev); > + if (ret != 0) { > + dev_err(&pdev->dev, "could not enable the pci device\n"); > + goto err_pci_en; > + } > + > + chip->mem_base = pci_resource_start(pdev, IO_MEM_BAR); > + if (!chip->mem_base) { > + dev_err(&pdev->dev, "could not locate IO memory address\n"); > + ret = -ENODEV; > + goto err_pci_start; > + } > + > + /* retrieve the available length of the IO memory space */ > + chip->mem_size = pci_resource_len(pdev, IO_MEM_BAR); > + > + /* allocate the memory for the device registers */ > + if (!request_mem_region(chip->mem_base, chip->mem_size, "1588_regs")) { > + dev_err(&pdev->dev, > + "could not allocate register memory space\n"); > + ret = -EBUSY; > + goto err_req_mem_region; > + } > + > + /* get the virtual address to the 1588 registers */ > + chip->regs = ioremap(chip->mem_base, chip->mem_size); > + > + if (!chip->regs) { > + dev_err(&pdev->dev, "Could not get virtual address\n"); > + ret = -ENOMEM; > + goto err_ioremap; > + } > + > + chip->caps = ptp_pch_caps; > + chip->ptp_clock = ptp_clock_register(&chip->caps); > + > + if (IS_ERR(chip->ptp_clock)) > + return PTR_ERR(chip->ptp_clock); > + > + spin_lock_init(&chip->register_lock); > + > + ret = request_irq(pdev->irq, &isr, IRQF_SHARED, KBUILD_MODNAME, chip); > + if (ret != 0) { > + dev_err(&pdev->dev, "failed to get irq %d\n", pdev->irq); > + goto err_req_irq; > + } > + > + /* indicate success */ > + chip->irq = pdev->irq; > + chip->pdev = pdev; > + pci_set_drvdata(pdev, chip); > + > + spin_lock_irqsave(&chip->register_lock, flags); > + /* reset the ieee1588 h/w */ > + pch_reset(chip); > + > + iowrite32(DEFAULT_ADDEND, &chip->regs->addend); > + iowrite32(1, &chip->regs->trgt_lo); > + iowrite32(0, &chip->regs->trgt_hi); > + iowrite32(PCH_TSE_TTIPEND, &chip->regs->event); > + /* Version: IEEE1588 v1 and IEEE1588-2008, Mode: All Evwnt, Locked */ > + iowrite32(0x80020000, &chip->regs->ch_control); Don't use a magic number. Instead, use the nice macros #define V2_MODE (1<<31) #define CAP_MODE2 (1<<17) from the next patch. > + > + pch_eth_enable_set(chip); > + > + if (strcmp(pch_param.station, "00:00:00:00:00:00") != 0) { > + if (pch_set_station_address(pch_param.station, pdev) != 0) { > + dev_err(&pdev->dev, > + "Invalid station address parameter\n" > + "Module loaded but station address not set correctly\n" > + ); > + } > + } > + spin_unlock_irqrestore(&chip->register_lock, flags); > + return 0; > + > +err_req_irq: > + ptp_clock_unregister(chip->ptp_clock); > + iounmap(chip->regs); > + chip->regs = 0; > + > +err_ioremap: > + release_mem_region(chip->mem_base, chip->mem_size); > + > +err_req_mem_region: > + chip->mem_base = 0; > + > +err_pci_start: > + pci_disable_device(pdev); > + > +err_pci_en: > + kfree(chip); > + dev_err(&pdev->dev, "probe failed(ret=0x%x)\n", ret); > + > + return ret; > +} > + > +static DEFINE_PCI_DEVICE_TABLE(pch_ieee1588_pcidev_id) = { > + { > + .vendor = PCI_VENDOR_ID_INTEL, > + .device = PCI_DEVICE_ID_PCH_1588 > + }, > + {0} > +}; > + > +static struct pci_driver pch_pcidev = { > + .name = KBUILD_MODNAME, > + .id_table = pch_ieee1588_pcidev_id, > + .probe = pch_probe, > + .remove = pch_remove, > + .suspend = pch_suspend, > + .resume = pch_resume, > +}; > + > +static void __exit ptp_pch_exit(void) > +{ > + pci_unregister_driver(&pch_pcidev); > +} > + > +static s32 __init ptp_pch_init(void) > +{ > + s32 ret; > + > + /* register the driver with the pci core */ > + ret = pci_register_driver(&pch_pcidev); > + > + return ret; > +} > + > +module_init(ptp_pch_init); > +module_exit(ptp_pch_exit); > + > +module_param_string(station, pch_param.station, sizeof pch_param.station, 0444); > +MODULE_PARM_DESC(station, > + "IEEE 1588 station address to use - column separated hex values"); > + > +MODULE_AUTHOR("LAPIS SEMICONDUCTOR, "); > +MODULE_DESCRIPTION("PTP clock using the EG20T timer"); > +MODULE_LICENSE("GPL"); > + > -- > 1.7.4.4 >