From: Randy Dunlap on
On Wed, 30 Jun 2010 16:51:30 +0900 Masayuki Ohtak wrote:

> Hi Andy and Andrew
>
> I have modified for your comments.
> Please confirm below.
>
> ---
>
>
> Packet hub driver of Topcliff PCH
>
> Topcliff PCH is the platform controller hub that is going to be used in
> Intel's upcoming general embedded platform. All IO peripherals in
> Topcliff PCH are actually devices sitting on AMBA bus. Packet hub is
> a special converter device in Topcliff PCH that translate AMBA transactions
> to PCI Express transactions and vice versa. Thus packet hub helps present
> all IO peripherals in Topcliff PCH as PCIE devices to IA system.
> Topcliff PCH have MAC address and Option ROM data.
> These data are in SROM which is connected to PCIE bus.
> Packet hub driver of Topcliff PCH can access MAC address and Option ROM data in
> SROM.
>
> Signed-off-by: Masayuki Ohtake <masa-korg(a)dsn.okisemi.com>
> Acked-by: Arnd <Arnd's email address>

It's listed in the MAINTAINERS file. Please use that.


>
> diff --git a/drivers/char/Kconfig b/drivers/char/Kconfig
> index e023682..1851e97 100644
> --- a/drivers/char/Kconfig
> +++ b/drivers/char/Kconfig
> @@ -4,6 +4,16 @@
>
> menu "Character devices"
>
> +config PCH_PHUB
> + tristate "PCH PHUB"
> + depends on PCI
> + help
> + Topcliff is a IOH for x86 embedded processor. This IOH is quite

is an IOH for x86 embedded processors.

> + different with the traditional IOH. Topcliff is a kind of ARM-based

from

> + processor and connected with PCIe bus (from x86 processor).
> + PHUB work as a gateway transform the PCIe transaction into the AMBA

works as a gateway to transform

> + transaction, and vise verse, and have several transform windows also.

vice versa, and has several



Maybe lose some acromyns? Overall, this help text should be about
*what* PCH_PHUB is, not *how* it accomplishes its work.
If you want to document *how* it works, put that into the source code
or Documentation/ somewhere.


---
~Randy
*** Remember to use Documentation/SubmitChecklist when testing your code ***
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Masayuki Ohtake on
Hi Randy

> It's listed in the MAINTAINERS file. Please use that.
I will add Arnd's e-mail address.

> Maybe lose some acromyns? Overall, this help text should be about
> *what* PCH_PHUB is, not *how* it accomplishes its work.
> If you want to document *how* it works, put that into the source code
> or Documentation/ somewhere.
I will rewrite my help text.


Thanks, Ohtake

----- Original Message -----
From: "Randy Dunlap" <randy.dunlap(a)oracle.com>
To: "Masayuki Ohtak" <masa-korg(a)dsn.okisemi.com>
Cc: "Andy Isaacson" <adi(a)hexapodia.org>; "Andrew Morton" <akpm(a)linux-foundation.org>; "Arnd Bergmann" <arnd(a)arndb.de>;
"Wang, Yong Y" <yong.y.wang(a)intel.com>; <qi.wang(a)intel.com>; <joel.clark(a)intel.com>; <andrew.chih.howe.khor(a)intel.com>;
"Alan Cox" <alan(a)lxorguk.ukuu.org.uk>; "LKML" <linux-kernel(a)vger.kernel.org>
Sent: Thursday, July 01, 2010 3:05 AM
Subject: Re: [PATCH] Packet hub driver of Topcliff PCH


> On Wed, 30 Jun 2010 16:51:30 +0900 Masayuki Ohtak wrote:
>
> > Hi Andy and Andrew
> >
> > I have modified for your comments.
> > Please confirm below.
> >
> > ---
> >
> >
> > Packet hub driver of Topcliff PCH
> >
> > Topcliff PCH is the platform controller hub that is going to be used in
> > Intel’s upcoming general embedded platform. All IO peripherals in
> > Topcliff PCH are actually devices sitting on AMBA bus. Packet hub is
> > a special converter device in Topcliff PCH that translate AMBA transactions
> > to PCI Express transactions and vice versa. Thus packet hub helps present
> > all IO peripherals in Topcliff PCH as PCIE devices to IA system.
> > Topcliff PCH have MAC address and Option ROM data.
> > These data are in SROM which is connected to PCIE bus.
> > Packet hub driver of Topcliff PCH can access MAC address and Option ROM data in
> > SROM.
> >
> > Signed-off-by: Masayuki Ohtake <masa-korg(a)dsn.okisemi.com>
> > Acked-by: Arnd <Arnd’s email address>
>
> It's listed in the MAINTAINERS file. Please use that.
>
>
> >
> > diff --git a/drivers/char/Kconfig b/drivers/char/Kconfig
> > index e023682..1851e97 100644
> > --- a/drivers/char/Kconfig
> > +++ b/drivers/char/Kconfig
> > @@ -4,6 +4,16 @@
> >
> > menu "Character devices"
> >
> > +config PCH_PHUB
> > + tristate "PCH PHUB"
> > + depends on PCI
> > + help
> > + Topcliff is a IOH for x86 embedded processor. This IOH is quite
>
> is an IOH for x86 embedded processors.
>
> > + different with the traditional IOH. Topcliff is a kind of ARM-based
>
> from
>
> > + processor and connected with PCIe bus (from x86 processor).
> > + PHUB work as a gateway transform the PCIe transaction into the AMBA
>
> works as a gateway to transform
>
> > + transaction, and vise verse, and have several transform windows also.
>
> vice versa, and has several
>
>
>
> Maybe lose some acromyns? Overall, this help text should be about
> *what* PCH_PHUB is, not *how* it accomplishes its work.
> If you want to document *how* it works, put that into the source code
> or Documentation/ somewhere.
>
>
> ---
> ~Randy
> *** Remember to use Documentation/SubmitChecklist when testing your code ***
>


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Andy Isaacson on
On Thu, Jul 01, 2010 at 02:14:11PM +0900, Masayuki Ohtak wrote:
> Hi Andy and Randy
>
> I have modified for your comments.
> Please confirm below.

Your style is looking better, and the additional documentation is much
appreciated.

I still have concerns about the userland interface design. It still
seems to me that the MAC interface should be used by something in
drivers/net and there's no reason to expose the SROM in /dev unless it
contains more than just the MAC address.


A few more style issues I saw on a quick scan through -- this was not a
comprehensive review:


> +static long pch_phub_ioctl(struct file *file, unsigned int cmd,
> + unsigned long arg)
[snip]
> + ret_value = copy_to_user(varg,
> + mac_addr, sizeof(mac_addr));

Reformat this to fit on one line:
+ ret_value = copy_to_user(varg, mac_addr, sizeof(mac_addr));


[snip]
> + case IOCTL_PHUB_WRITE_MAC_ADDR:
> + ret_value = copy_from_user(mac_addr, varg, sizeof(mac_addr));
> +
> + if (ret_value)

Here we need to break:

+ {
> + ret_value = -EFAULT;
+ break;
+ }

because if copy_from_user failed ...

> + for (i = 0; i < 6; i++)
> + pch_phub_write_gbe_mac_addr(i, mac_addr[i]);

.... we would pass garbage to pch_phub_write_gbe_mac_addr.

[snip]
> + dev_dbg(&pdev->dev,
> + "\npch_phub_probe : pci_enable_device FAILED");

Prefix \n is not correct. This should be

+ dev_dbg(&pdev->dev, "pch_phub_probe: pci_enable_device FAILED");

In general dev_dbg format strings should fit on one 80-char line. If
your format strings are longer than that, it's a clue you're doing
something wrong. For example:

> + dev_dbg(&pdev->dev, "pch_phub_probe : "
> + "pci_enable_device returns %d\n", ret);

+ dev_dbg(&pdev->dev, "pch_phub_probe: pci_enable_device returns %d\n",
+ ret);

> + ret = pci_request_regions(pdev, MODULE_NAME);
> + if (ret) {
> + dev_dbg(&pdev->dev,
> + "pch_phub_probe : pci_request_regions FAILED");

If you have a dev_dbg, please print ret. It may give an important clue.

[snip a bunch more I don't have time to review right now]

Thanks,
-andy
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Masayuki Ohtake on
Hi Andy,

> I still have concerns about the userland interface design. It still
> seems to me that the MAC interface should be used by something in
> drivers/net and there's no reason to expose the SROM in /dev unless it
> contains more than just the MAC address.
SROM has not only MAC address but also Option ROM data.
Thus, we think SROM should be exposed.

> A few more style issues I saw on a quick scan through -- this was not a
> comprehensive review:
We are now modifying.

Thanks, Ohtake

----- Original Message -----
From: "Andy Isaacson" <adi(a)hexapodia.org>
To: "Masayuki Ohtak" <masa-korg(a)dsn.okisemi.com>
Cc: "Randy Dunlap" <randy.dunlap(a)oracle.com>; "Arnd Bergmann" <arnd(a)arndb.de>; "Wang, Yong Y" <yong.y.wang(a)intel.com>;
<qi.wang(a)intel.com>; <joel.clark(a)intel.com>; <andrew.chih.howe.khor(a)intel.com>; "Alan Cox" <alan(a)lxorguk.ukuu.org.uk>;
"LKML" <linux-kernel(a)vger.kernel.org>
Sent: Thursday, July 01, 2010 3:58 PM
Subject: Re: [PATCH] Packet hub driver of Topcliff PCH


> On Thu, Jul 01, 2010 at 02:14:11PM +0900, Masayuki Ohtak wrote:
> > Hi Andy and Randy
> >
> > I have modified for your comments.
> > Please confirm below.
>
> Your style is looking better, and the additional documentation is much
> appreciated.
>
> I still have concerns about the userland interface design. It still
> seems to me that the MAC interface should be used by something in
> drivers/net and there's no reason to expose the SROM in /dev unless it
> contains more than just the MAC address.
>
>
> A few more style issues I saw on a quick scan through -- this was not a
> comprehensive review:
>
>
> > +static long pch_phub_ioctl(struct file *file, unsigned int cmd,
> > + unsigned long arg)
> [snip]
> > + ret_value = copy_to_user(varg,
> > + mac_addr, sizeof(mac_addr));
>
> Reformat this to fit on one line:
> + ret_value = copy_to_user(varg, mac_addr, sizeof(mac_addr));
>
>
> [snip]
> > + case IOCTL_PHUB_WRITE_MAC_ADDR:
> > + ret_value = copy_from_user(mac_addr, varg, sizeof(mac_addr));
> > +
> > + if (ret_value)
>
> Here we need to break:
>
> + {
> > + ret_value = -EFAULT;
> + break;
> + }
>
> because if copy_from_user failed ...
>
> > + for (i = 0; i < 6; i++)
> > + pch_phub_write_gbe_mac_addr(i, mac_addr[i]);
>
> ... we would pass garbage to pch_phub_write_gbe_mac_addr.
>
> [snip]
> > + dev_dbg(&pdev->dev,
> > + "\npch_phub_probe : pci_enable_device FAILED");
>
> Prefix \n is not correct. This should be
>
> + dev_dbg(&pdev->dev, "pch_phub_probe: pci_enable_device FAILED");
>
> In general dev_dbg format strings should fit on one 80-char line. If
> your format strings are longer than that, it's a clue you're doing
> something wrong. For example:
>
> > + dev_dbg(&pdev->dev, "pch_phub_probe : "
> > + "pci_enable_device returns %d\n", ret);
>
> + dev_dbg(&pdev->dev, "pch_phub_probe: pci_enable_device returns %d\n",
> + ret);
>
> > + ret = pci_request_regions(pdev, MODULE_NAME);
> > + if (ret) {
> > + dev_dbg(&pdev->dev,
> > + "pch_phub_probe : pci_request_regions FAILED");
>
> If you have a dev_dbg, please print ret. It may give an important clue.
>
> [snip a bunch more I don't have time to review right now]
>
> Thanks,
> -andy
>


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Randy Dunlap on
On 07/01/10 03:38, Masayuki Ohtak wrote:
> Packet hub driver of Topcliff PCH
>
> Topcliff PCH is the platform controller hub that is going to be used in
> Intel's upcoming general embedded platform. All IO peripherals in
> Topcliff PCH are actually devices sitting on AMBA bus. Packet hub is
> a special converter device in Topcliff PCH that translate AMBA transactions
> to PCI Express transactions and vice versa. Thus packet hub helps present
> all IO peripherals in Topcliff PCH as PCIE devices to IA system.
> Topcliff PCH have MAC address and Option ROM data.
> These data are in SROM which is connected to PCIE bus.
> Packet hub driver of Topcliff PCH can access MAC address and Option ROM data in
> SROM.
>
> Signed-off-by: Masayuki Ohtake <masa-korg(a)dsn.okisemi.com>
> Acked-by: Arnd Bergmann <arnd(a)arndb.de>
>
> ---
> drivers/char/Kconfig | 9 +
> drivers/char/Makefile | 2 +
> drivers/char/pch_phub/Makefile | 3 +
> drivers/char/pch_phub/pch_phub.c | 811 ++++++++++++++++++++++++++++++++++++++
> drivers/char/pch_phub/pch_phub.h | 48 +++
> 5 files changed, 873 insertions(+), 0 deletions(-)
> create mode 100644 drivers/char/pch_phub/Makefile
> create mode 100755 drivers/char/pch_phub/pch_phub.c
> create mode 100755 drivers/char/pch_phub/pch_phub.h
>
> diff --git a/drivers/char/Kconfig b/drivers/char/Kconfig
> index e023682..9ef3c85 100644
> --- a/drivers/char/Kconfig
> +++ b/drivers/char/Kconfig
> @@ -4,6 +4,15 @@
>
> menu "Character devices"
>
> +config PCH_PHUB
> + tristate "PCH PHUB"
> + depends on PCI
> + help
> + This driver is for PCH PHUB of Topcliff which is an IOH for x86
> + embedded processor. The Topcliff has MAC address and Option ROM data
> + in SROM. This dirver can access MAC address and Option ROM data in

driver

> + SROM.
> +
> config VT
> bool "Virtual terminal" if EMBEDDED
> depends on !S390

> diff --git a/drivers/char/pch_phub/pch_phub.c b/drivers/char/pch_phub/pch_phub.c
> new file mode 100755
> index 0000000..9a0f8a9
> --- /dev/null
> +++ b/drivers/char/pch_phub/pch_phub.c
> @@ -0,0 +1,811 @@
> +/*!

What is the ! for?

> + * Copyright (C) 2010 OKI SEMICONDUCTOR Co., LTD.
> + *
> + * 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 <linux/module.h>
> +#include <linux/kernel.h>
> +#include <linux/types.h>
> +#include <linux/fs.h>
> +#include <linux/uaccess.h>
> +#include <linux/string.h>
> +#include <linux/pci.h>
> +#include <linux/io.h>
> +#include <linux/delay.h>
> +#include <linux/cdev.h>
> +#include <linux/device.h>
> +#include <linux/mutex.h>
> +#include "pch_phub.h"
> +
> +#define PHUB_STATUS 0x00 /* Status Register offset */
> +#define PHUB_CONTROL 0x04 /* Control Register offset */
> +#define PHUB_TIMEOUT 0x05 /* Time out value for Status Register */
> +#define PCH_PHUB_ROM_WRITE_ENABLE 0x01 /* Enabling for writing ROM */
> +#define PCH_PHUB_ROM_WRITE_DISABLE 0x00 /* Disabling for writing ROM */
> +#define PCH_PHUB_ROM_START_ADDR 0x14 /* ROM data area start address offset */
> +
> +/* MAX number of INT_REDUCE_CONTROL registers */
> +#define MAX_NUM_INT_REDUCE_CONTROL_REG 128
> +#define PCI_DEVICE_ID_PCH1_PHUB 0x8801
> +#define PCH_MINOR_NOS 1
> +#define CLKCFG_CAN_50MHZ 0x12000000
> +#define CLKCFG_CANCLK_MASK 0xFF000000
> +#define MODULE_NAME "pch_phub"
> +
> +/**
> + * struct pch_phub_reg - PHUB register structure
> + * @phub_id_reg: PHUB_ID register val
> + * @q_pri_val_reg: QUEUE_PRI_VAL register val
> + * @rc_q_maxsize_reg: RC_QUEUE_MAXSIZE register val
> + * @bri_q_maxsize_reg: BRI_QUEUE_MAXSIZE register val
> + * @comp_resp_timeout_reg: COMP_RESP_TIMEOUT register val
> + * @bus_slave_control_reg: BUS_SLAVE_CONTROL_REG register val
> + * @deadlock_avoid_type_reg: DEADLOCK_AVOID_TYPE register val
> + * @intpin_reg_wpermit_reg0: INTPIN_REG_WPERMIT register 0 val
> + * @intpin_reg_wpermit_reg1: INTPIN_REG_WPERMIT register 1 val
> + * @intpin_reg_wpermit_reg2: INTPIN_REG_WPERMIT register 2 val
> + * @intpin_reg_wpermit_reg3: INTPIN_REG_WPERMIT register 3 val
> + * @int_reduce_control_reg: INT_REDUCE_CONTROL registers val
> + * @clkcfg_reg: CLK CFG register val
> + * @pch_phub_base_address: Register base address
> + * @pch_phub_extrom_base_address: external rom base address
> + * @pch_phub_suspended: PHUB status val
> + */
> +struct pch_phub_reg {
> + u32 phub_id_reg;
> + u32 q_pri_val_reg;
> + u32 rc_q_maxsize_reg;
> + u32 bri_q_maxsize_reg;
> + u32 comp_resp_timeout_reg;
> + u32 bus_slave_control_reg;
> + u32 deadlock_avoid_type_reg;
> + u32 intpin_reg_wpermit_reg0;
> + u32 intpin_reg_wpermit_reg1;
> + u32 intpin_reg_wpermit_reg2;
> + u32 intpin_reg_wpermit_reg3;
> + u32 int_reduce_control_reg[MAX_NUM_INT_REDUCE_CONTROL_REG];
> + u32 clkcfg_reg;
> + void __iomem *pch_phub_base_address;
> + void __iomem *pch_phub_extrom_base_address;
> + int pch_phub_suspended;
> +} pch_phub_reg;
> +
> +/* SROM SPEC for MAC address assignment offset */
> +static const int pch_phub_mac_offset[6] = {0x3, 0x2, 0x1, 0x0, 0xb, 0xa};
> +
> +static DEFINE_MUTEX(pch_phub_ioctl_mutex);
> +static DEFINE_MUTEX(pch_phub_read_mutex);
> +static DEFINE_MUTEX(pch_phub_write_mutex);
> +static dev_t pch_phub_dev_no;
> +static struct cdev pch_phub_dev;
> +
> +/**
> + * pch_phub_read_modify_write_reg() - Reading modifying and writing register
> + * @reg_addr_offset: Register offset address value.
> + * @data: Writing value.
> + * @mask: Mask value.
> + */
> +static void pch_phub_read_modify_write_reg(unsigned int reg_addr_offset,
> + unsigned int data, unsigned int mask)
> +{
> + void __iomem *reg_addr = pch_phub_reg.pch_phub_base_address +
> + reg_addr_offset;
> + iowrite32(((ioread32(reg_addr) & ~mask)) | data, reg_addr);
> +}
> +
> +
> +/* pch_phub_save_reg_conf - saves register configuration */
> +static void pch_phub_save_reg_conf(struct pci_dev *pdev)
> +{
> + unsigned int i;
> + void __iomem *p = pch_phub_reg.pch_phub_base_address;
> +
> + dev_dbg(&pdev->dev, "pch_phub_save_reg_conf ENTRY\n");
> + pch_phub_reg.phub_id_reg = ioread32(p + PCH_PHUB_ID_REG);
> + pch_phub_reg.q_pri_val_reg = ioread32(p + PCH_PHUB_QUEUE_PRI_VAL_REG);
> + pch_phub_reg.rc_q_maxsize_reg =
> + ioread32(p + PCH_PHUB_RC_QUEUE_MAXSIZE_REG);
> + pch_phub_reg.bri_q_maxsize_reg =
> + ioread32(p + PCH_PHUB_BRI_QUEUE_MAXSIZE_REG);
> + pch_phub_reg.comp_resp_timeout_reg =
> + ioread32(p + PCH_PHUB_COMP_RESP_TIMEOUT_REG);
> + pch_phub_reg.bus_slave_control_reg =
> + ioread32(p + PCH_PHUB_BUS_SLAVE_CONTROL_REG);
> + pch_phub_reg.deadlock_avoid_type_reg =
> + ioread32(p + PCH_PHUB_DEADLOCK_AVOID_TYPE_REG);
> + pch_phub_reg.intpin_reg_wpermit_reg0 =
> + ioread32(p + PCH_PHUB_INTPIN_REG_WPERMIT_REG0);
> + pch_phub_reg.intpin_reg_wpermit_reg1 =
> + ioread32(p + PCH_PHUB_INTPIN_REG_WPERMIT_REG1);
> + pch_phub_reg.intpin_reg_wpermit_reg2 =
> + ioread32(p + PCH_PHUB_INTPIN_REG_WPERMIT_REG2);
> + pch_phub_reg.intpin_reg_wpermit_reg3 =
> + ioread32(p + PCH_PHUB_INTPIN_REG_WPERMIT_REG3);
> + dev_dbg(&pdev->dev, "pch_phub_save_reg_conf : "
> + "pch_phub_reg.phub_id_reg=%x, "
> + "pch_phub_reg.q_pri_val_reg=%x, "
> + "pch_phub_reg.rc_q_maxsize_reg=%x, "
> + "pch_phub_reg.bri_q_maxsize_reg=%x, "
> + "pch_phub_reg.comp_resp_timeout_reg=%x, "
> + "pch_phub_reg.bus_slave_control_reg=%x, "
> + "pch_phub_reg.deadlock_avoid_type_reg=%x, "
> + "pch_phub_reg.intpin_reg_wpermit_reg0=%x, "
> + "pch_phub_reg.intpin_reg_wpermit_reg1=%x, "
> + "pch_phub_reg.intpin_reg_wpermit_reg2=%x, "
> + "pch_phub_reg.intpin_reg_wpermit_reg3=%x\n",
> + pch_phub_reg.phub_id_reg,
> + pch_phub_reg.q_pri_val_reg,
> + pch_phub_reg.rc_q_maxsize_reg,
> + pch_phub_reg.bri_q_maxsize_reg,
> + pch_phub_reg.comp_resp_timeout_reg,
> + pch_phub_reg.bus_slave_control_reg,
> + pch_phub_reg.deadlock_avoid_type_reg,
> + pch_phub_reg.intpin_reg_wpermit_reg0,
> + pch_phub_reg.intpin_reg_wpermit_reg1,
> + pch_phub_reg.intpin_reg_wpermit_reg2,
> + pch_phub_reg.intpin_reg_wpermit_reg3);
> + for (i = 0; i < MAX_NUM_INT_REDUCE_CONTROL_REG; i++) {
> + pch_phub_reg.int_reduce_control_reg[i] =
> + ioread32(p + PCH_PHUB_INT_REDUCE_CONTROL_REG_BASE + 4 * i);
> + dev_dbg(&pdev->dev, "pch_phub_save_reg_conf : "
> + "pch_phub_reg.int_reduce_control_reg[%d]=%x\n",
> + i, pch_phub_reg.int_reduce_control_reg[i]);
> + }
> + pch_phub_reg.clkcfg_reg = ioread32(p + CLKCFG_REG_OFFSET);
> +

drop blank line above.

> +}
> +
> +/* pch_phub_restore_reg_conf - restore register configuration */
> +static void pch_phub_restore_reg_conf(struct pci_dev *pdev)
> +{
> + unsigned int i;
> + void __iomem *p = pch_phub_reg.pch_phub_base_address;
> +
> + dev_dbg(&pdev->dev, "pch_phub_restore_reg_conf ENTRY\n");
> + iowrite32(pch_phub_reg.phub_id_reg, p + PCH_PHUB_ID_REG);
> + iowrite32(pch_phub_reg.q_pri_val_reg, p + PCH_PHUB_QUEUE_PRI_VAL_REG);
> + iowrite32(pch_phub_reg.rc_q_maxsize_reg,
> + p + PCH_PHUB_RC_QUEUE_MAXSIZE_REG);
> + iowrite32(pch_phub_reg.bri_q_maxsize_reg,
> + p + PCH_PHUB_BRI_QUEUE_MAXSIZE_REG);
> + iowrite32(pch_phub_reg.comp_resp_timeout_reg,
> + p + PCH_PHUB_COMP_RESP_TIMEOUT_REG);
> + iowrite32(pch_phub_reg.bus_slave_control_reg,
> + p + PCH_PHUB_BUS_SLAVE_CONTROL_REG);
> + iowrite32(pch_phub_reg.deadlock_avoid_type_reg,
> + p + PCH_PHUB_DEADLOCK_AVOID_TYPE_REG);
> + iowrite32(pch_phub_reg.intpin_reg_wpermit_reg0,
> + p + PCH_PHUB_INTPIN_REG_WPERMIT_REG0);
> + iowrite32(pch_phub_reg.intpin_reg_wpermit_reg1,
> + p + PCH_PHUB_INTPIN_REG_WPERMIT_REG1);
> + iowrite32(pch_phub_reg.intpin_reg_wpermit_reg2,
> + p + PCH_PHUB_INTPIN_REG_WPERMIT_REG2);
> + iowrite32(pch_phub_reg.intpin_reg_wpermit_reg3,
> + p + PCH_PHUB_INTPIN_REG_WPERMIT_REG3);
> + dev_dbg(&pdev->dev, "pch_phub_save_reg_conf : "
> + "pch_phub_reg.phub_id_reg=%x, "
> + "pch_phub_reg.q_pri_val_reg=%x, "
> + "pch_phub_reg.rc_q_maxsize_reg=%x, "
> + "pch_phub_reg.bri_q_maxsize_reg=%x, "
> + "pch_phub_reg.comp_resp_timeout_reg=%x, "
> + "pch_phub_reg.bus_slave_control_reg=%x, "
> + "pch_phub_reg.deadlock_avoid_type_reg=%x, "
> + "pch_phub_reg.intpin_reg_wpermit_reg0=%x, "
> + "pch_phub_reg.intpin_reg_wpermit_reg1=%x, "
> + "pch_phub_reg.intpin_reg_wpermit_reg2=%x, "
> + "pch_phub_reg.intpin_reg_wpermit_reg3=%x\n",
> + pch_phub_reg.phub_id_reg,
> + pch_phub_reg.q_pri_val_reg,
> + pch_phub_reg.rc_q_maxsize_reg,
> + pch_phub_reg.bri_q_maxsize_reg,
> + pch_phub_reg.comp_resp_timeout_reg,
> + pch_phub_reg.bus_slave_control_reg,
> + pch_phub_reg.deadlock_avoid_type_reg,
> + pch_phub_reg.intpin_reg_wpermit_reg0,
> + pch_phub_reg.intpin_reg_wpermit_reg1,
> + pch_phub_reg.intpin_reg_wpermit_reg2,
> + pch_phub_reg.intpin_reg_wpermit_reg3);
> + for (i = 0; i < MAX_NUM_INT_REDUCE_CONTROL_REG; i++) {
> + iowrite32(pch_phub_reg.int_reduce_control_reg[i],
> + p + PCH_PHUB_INT_REDUCE_CONTROL_REG_BASE + 4 * i);
> + dev_dbg(&pdev->dev, "pch_phub_save_reg_conf : "
> + "pch_phub_reg.int_reduce_control_reg[%d]=%x\n",
> + i, pch_phub_reg.int_reduce_control_reg[i]);
> + }
> +
> + iowrite32(pch_phub_reg.clkcfg_reg, p + CLKCFG_REG_OFFSET);
> +}
> +
> +/**
> + * pch_phub_read_serial_rom() - Reading Serial ROM
> + * @offset_address: Serial ROM offset address to read.
> + * @data: Read buffer for specified Serial ROM value.
> + */
> +static void pch_phub_read_serial_rom(unsigned int offset_address, u8 *data)
> +{
> + void __iomem *mem_addr = pch_phub_reg.pch_phub_extrom_base_address +
> + offset_address;
> +
> + *data = ioread8(mem_addr);
> +}
> +
> +/**
> + * pch_phub_write_serial_rom() - Writing Serial ROM
> + * @offset_address: Serial ROM offset address.
> + * @data: Serial ROM value to write.
> + */
> +static int pch_phub_write_serial_rom(unsigned int offset_address, u8 data)
> +{
> + void __iomem *mem_addr = pch_phub_reg.pch_phub_extrom_base_address +
> + (offset_address & PCH_WORD_ADDR_MASK);
> + int i;
> + unsigned int word_data;
> + unsigned int pos;
> + unsigned int mask;
> + pos = (offset_address % 4) * 8;
> + mask = ~(0xFF << pos);
> +
> + iowrite32(PCH_PHUB_ROM_WRITE_ENABLE,
> + pch_phub_reg.pch_phub_extrom_base_address +
> + PHUB_CONTROL);
> +
> + word_data = ioread32(mem_addr);
> + iowrite32((word_data & mask) | (u32)data << pos, mem_addr);
> +
> + i = 0;
> + while (0x00 != ioread8(pch_phub_reg.pch_phub_extrom_base_address +
> + PHUB_STATUS)) {

We prefer to have constant on right side of comparison...

> + msleep(1);
> + if (PHUB_TIMEOUT == i)
> + return -EPERM;
> + i++;
> + }
> +
> + iowrite32(PCH_PHUB_ROM_WRITE_DISABLE,
> + pch_phub_reg.pch_phub_extrom_base_address +
> + PHUB_CONTROL);
> +
> + return 0;
> +}
> +
> +/**
> + * pch_phub_read_serial_rom_val() - Read Serial ROM value
> + * @offset_address: Serial ROM address offset value.
> + * @data: Serial ROM value to read.
> + */
> +static void pch_phub_read_serial_rom_val(unsigned int offset_address, u8 *data)
> +{
> + unsigned int mem_addr;
> +
> + mem_addr = PCH_PHUB_ROM_START_ADDR +
> + pch_phub_mac_offset[offset_address];
> +
> + pch_phub_read_serial_rom(mem_addr, data);
> +

drop blank line above.

> +}
> +
> +
> +/**
> + * pch_phub_write_serial_rom_val() - writing Serial ROM value
> + * @offset_address: Serial ROM address offset value.
> + * @data: Serial ROM value.
> + */
> +static int pch_phub_write_serial_rom_val(unsigned int offset_address, u8 data)
> +{
> + int retval;
> + unsigned int mem_addr;
> +
> + mem_addr = PCH_PHUB_ROM_START_ADDR +
> + pch_phub_mac_offset[offset_address];
> +
> + retval = pch_phub_write_serial_rom(mem_addr, data);
> +
> + return retval;
> +}
> +
> +/* pch_phub_gbe_serial_rom_conf - makes Serial ROM header format configuration
> + * for Gigabit Ethernet MAC address
> + */
> +static int pch_phub_gbe_serial_rom_conf(void)
> +{
> + int retval;
> +
> + retval = pch_phub_write_serial_rom(0x0b, 0xbc);
> + retval |= pch_phub_write_serial_rom(0x0a, 0x10);
> + retval |= pch_phub_write_serial_rom(0x09, 0x01);
> + retval |= pch_phub_write_serial_rom(0x08, 0x02);
> +
> + retval |= pch_phub_write_serial_rom(0x0f, 0x00);
> + retval |= pch_phub_write_serial_rom(0x0e, 0x00);
> + retval |= pch_phub_write_serial_rom(0x0d, 0x00);
> + retval |= pch_phub_write_serial_rom(0x0c, 0x80);
> +
> + retval |= pch_phub_write_serial_rom(0x13, 0xbc);
> + retval |= pch_phub_write_serial_rom(0x12, 0x10);
> + retval |= pch_phub_write_serial_rom(0x11, 0x01);
> + retval |= pch_phub_write_serial_rom(0x10, 0x18);
> +
> + retval |= pch_phub_write_serial_rom(0x1b, 0xbc);
> + retval |= pch_phub_write_serial_rom(0x1a, 0x10);
> + retval |= pch_phub_write_serial_rom(0x19, 0x01);
> + retval |= pch_phub_write_serial_rom(0x18, 0x19);
> +
> + retval |= pch_phub_write_serial_rom(0x23, 0xbc);
> + retval |= pch_phub_write_serial_rom(0x22, 0x10);
> + retval |= pch_phub_write_serial_rom(0x21, 0x01);
> + retval |= pch_phub_write_serial_rom(0x20, 0x3a);
> +
> + retval |= pch_phub_write_serial_rom(0x27, 0x01);
> + retval |= pch_phub_write_serial_rom(0x26, 0x00);
> + retval |= pch_phub_write_serial_rom(0x25, 0x00);
> + retval |= pch_phub_write_serial_rom(0x24, 0x00);
> +
> + return retval;
> +}
> +
> +/**
> + * pch_phub_read_gbe_mac_addr() - Read Gigabit Ethernet MAC address
> + * @offset_address: Gigabit Ethernet MAC address offset value.
> + * @data: Buffer of the Gigabit Ethernet MAC address value.
> + */
> +static void pch_phub_read_gbe_mac_addr(u8 *data)
> +{
> + int i;
> + for (i = 0; i < 6; i++)

s/6/ETH_ALEN/
(see below)

> + pch_phub_read_serial_rom_val(i, &data[i]);
> +}
> +
> +/**
> + * pch_phub_write_gbe_mac_addr() - Write MAC address
> + * @offset_address: Gigabit Ethernet MAC address offset value.
> + * @data: Gigabit Ethernet MAC address value.
> + */
> +static int pch_phub_write_gbe_mac_addr(u8 *data)
> +{
> + int retval;
> + int i;
> +
> + retval = pch_phub_gbe_serial_rom_conf();
> + if (retval)
> + return retval;
> +
> + for (i = 0; i < 6; i++) {

s/6/ETH_ALEN/
(see below)

> + retval = pch_phub_write_serial_rom_val(i, data[i]);
> + if (retval)
> + return retval;
> + }
> +
> + return retval;
> +}
> +
> +static ssize_t pch_phub_read(struct file *file, char __user *buf, size_t size,
> + loff_t *ppos)
> +{
> + unsigned int rom_signature;
> + unsigned char rom_length;
> + int ret_value;
> + unsigned int tmp;
> + unsigned char data;
> + unsigned int addr_offset;
> + unsigned int orom_size;
> + int ret;
> + int err;
> + loff_t pos = *ppos;
> +
> + ret = mutex_lock_interruptible(&pch_phub_read_mutex);
> + if (ret) {
> + err = -ERESTARTSYS;
> + goto return_err_nomutex;
> + }
> +
> + /* Get Rom signature */
> + pch_phub_read_serial_rom(0x80, (unsigned char *)&rom_signature);
> + rom_signature &= 0xff;
> + pch_phub_read_serial_rom(0x81, (unsigned char *)&tmp);
> + rom_signature |= (tmp & 0xff) << 8;
> + if (rom_signature == 0xAA55) {
> + pch_phub_read_serial_rom(0x82, &rom_length);
> + orom_size = rom_length * 512;
> + if (orom_size < pos) {
> + addr_offset = 0;
> + goto return_ok;
> + }
> +
> + for (addr_offset = 0; addr_offset < size; addr_offset++) {
> + pch_phub_read_serial_rom(0x80 + addr_offset + pos,
> + &data);
> + ret_value = copy_to_user(&buf[addr_offset], &data, 1);
> + if (ret_value) {
> + err = -EFAULT;
> + goto return_err;
> + }
> +
> + if (orom_size < pos + addr_offset) {
> + *ppos += addr_offset;
> + goto return_ok;
> + }
> +
> + }
> + } else {
> + return -ENODATA;

Need to unlock mutex here also??

> + }
> + *ppos += addr_offset;
> +return_ok:
> + mutex_unlock(&pch_phub_read_mutex);
> + return addr_offset;
> +
> +return_err:
> + mutex_unlock(&pch_phub_read_mutex);
> +return_err_nomutex:
> + return err;
> +}
> +
> +
> +static ssize_t pch_phub_write(struct file *file, const char __user *buf,
> + size_t size, loff_t *ppos)
> +{
> + unsigned int data;
> + int ret_value1;
> + int ret_value2;
> + int err;
> + unsigned int addr_offset;
> + loff_t pos = *ppos;
> + int ret;
> +
> + ret = mutex_lock_interruptible(&pch_phub_write_mutex);
> + if (ret) {
> + err = -ERESTARTSYS;
> + goto return_err_nomutex;
> + }
> +
> + for (addr_offset = 0; addr_offset < size; addr_offset++) {
> + ret_value1 = get_user(data, &buf[addr_offset]);
> + if (ret_value1) {
> + err = -EFAULT;
> + goto return_err;
> + }
> +
> + ret_value2 = pch_phub_write_serial_rom(0x80 + addr_offset + pos,
> + data);
> + if (ret_value2) {
> + err = ret_value2;
> + goto return_err;
> + }
> +
> + if (PCH_PHUB_OROM_SIZE < pos + addr_offset) {
> + *ppos += addr_offset;
> + goto return_ok;
> + }
> +
> + }
> +
> + *ppos += addr_offset;
> +
> +return_ok:
> + mutex_unlock(&pch_phub_write_mutex);
> + return addr_offset;
> +
> +return_err:
> + mutex_unlock(&pch_phub_write_mutex);
> +return_err_nomutex:
> + return err;
> +}
> +
> +
> +static long pch_phub_ioctl(struct file *file, unsigned int cmd,
> + unsigned long arg)
> +{
> + int ret_value;
> + int ret;
> + int rtn;
> + __u8 mac_addr[6];

#include <linux/if_ether.h>

__u8 mac_addr[ETH_ALEN];


> + void __user *varg = (void __user *)arg;
> +
> + ret = mutex_lock_interruptible(&pch_phub_ioctl_mutex);
> + if (ret) {
> + ret_value = -ERESTARTSYS;
> + goto return_nomutex;
> + }
> +
> + if (pch_phub_reg.pch_phub_suspended == true) {
> + ret_value = -EBUSY;
> + goto return_ioctrl;
> + }
> +
> + switch (cmd) {
> + case IOCTL_PHUB_READ_MAC_ADDR:
> + pch_phub_read_gbe_mac_addr(mac_addr);
> +
> + ret_value = copy_to_user(varg, mac_addr, sizeof(mac_addr));

s/sizeof(mac_addr)/ETH_ALEN/

> + break;
> +
> + case IOCTL_PHUB_WRITE_MAC_ADDR:
> + rtn = copy_from_user(mac_addr, varg, sizeof(mac_addr));

ditto.

> +
> + if (rtn) {
> + ret_value = -EFAULT;
> + break;
> + }
> +
> + ret_value = pch_phub_write_gbe_mac_addr(mac_addr);
> + break;
> +
> + default:
> + ret_value = -EINVAL;
> + break;
> + }
> +return_ioctrl:
> + mutex_unlock(&pch_phub_ioctl_mutex);
> +return_nomutex:
> + return ret_value;
> +}


> +static int __devinit pch_phub_probe(struct pci_dev *pdev,
> + const struct pci_device_id *id)
> +{
> + int ret;
> + unsigned int rom_size;
> +
> + ret = pci_enable_device(pdev);
> + if (ret) {
> + dev_dbg(&pdev->dev,
> + "pch_phub_probe : pci_enable_device FAILED(ret=%d)", ret);
> + goto err_pci_enable_dev;
> + }
> + dev_dbg(&pdev->dev, "pch_phub_probe : pci_enable_device returns %d\n",
> + ret);
> +
> + ret = pci_request_regions(pdev, MODULE_NAME);
> + if (ret) {
> + dev_dbg(&pdev->dev,
> + "pch_phub_probe : pci_request_regions FAILED(ret=%d)", ret);
> + goto err_req_regions;
> + }
> + dev_dbg(&pdev->dev, "pch_phub_probe : "
> + "pci_request_regions returns %d\n", ret);
> +
> + pch_phub_reg.pch_phub_base_address = pci_iomap(pdev, 1, 0);
> +
> + if (pch_phub_reg.pch_phub_base_address == 0) {
> + dev_dbg(&pdev->dev, "pch_phub_probe : pci_iomap FAILED");
> + ret = -ENOMEM;
> + goto err_pci_iomap;
> + }
> + dev_dbg(&pdev->dev, "pch_phub_probe : pci_iomap SUCCESS and value "
> + "in pch_phub_base_address variable is 0x%08x\n",
> + (unsigned int)pch_phub_reg.pch_phub_base_address);
> +
> + pch_phub_reg.pch_phub_extrom_base_address =
> + pci_map_rom(pdev, &rom_size);
> + if (pch_phub_reg.pch_phub_extrom_base_address == 0) {
> + dev_dbg(&pdev->dev, "pch_phub_probe : pci_map_rom FAILED");
> + ret = -ENOMEM;
> + goto err_pci_map;
> + }
> + dev_dbg(&pdev->dev, "pch_phub_probe : "
> + "pci_map_rom SUCCESS and value in "
> + "pch_phub_extrom_base_address variable is 0x%08x\n",
> + (unsigned int)pch_phub_reg.pch_phub_extrom_base_address);
> +
> + ret = alloc_chrdev_region(&pch_phub_dev_no, 0,
> + PCH_MINOR_NOS, MODULE_NAME);
> + if (ret) {
> + dev_dbg(&pdev->dev, "pch_phub_probe : "
> + "alloc_chrdev_region FAILED(ret=%d)", ret);
> +
> + goto err_alloc_cdev;
> + }
> + dev_dbg(&pdev->dev, "pch_phub_probe : "
> + "alloc_chrdev_region returns %d\n", ret);
> +
> + cdev_init(&pch_phub_dev, &pch_phub_fops);
> + dev_dbg(&pdev->dev,
> + "pch_phub_probe : cdev_init invoked successfully\n");
> +
> + pch_phub_dev.owner = THIS_MODULE;
> + pch_phub_dev.ops = &pch_phub_fops;
> +
> + ret = cdev_add(&pch_phub_dev, pch_phub_dev_no, PCH_MINOR_NOS);
> + if (ret) {
> + dev_dbg(&pdev->dev, "pch_phub_probe : cdev_add FAILED(ret=%d)",
> + ret);
> + goto err_cdev_add;
> + }
> + dev_dbg(&pdev->dev, "pch_phub_probe : cdev_add returns %d\n", ret);
> +
> + /* set the clock config reg if CAN clock is 50Mhz */
> + dev_dbg(&pdev->dev, "pch_phub_probe : invoking "
> + "pch_phub_read_modify_write_reg "
> + "to set CLKCFG reg for CAN clk 50Mhz\n");
> + pch_phub_read_modify_write_reg((unsigned int)CLKCFG_REG_OFFSET,
> + CLKCFG_CAN_50MHZ, CLKCFG_CANCLK_MASK);
> +
> + /* set the prefech value */
> + iowrite32(0x000ffffa, pch_phub_reg.pch_phub_base_address + 0x14);
> + /* set the interrupt delay value */
> + iowrite32(0x25, pch_phub_reg.pch_phub_base_address + 0x44);
> + return 0;
> +
> +err_cdev_add:
> + unregister_chrdev_region(pch_phub_dev_no, PCH_MINOR_NOS);
> +err_alloc_cdev:
> + pci_unmap_rom(pdev, pch_phub_reg.pch_phub_extrom_base_address);
> +err_pci_map:
> + pci_iounmap(pdev, pch_phub_reg.pch_phub_base_address);
> +err_pci_iomap:
> + pci_release_regions(pdev);
> +err_req_regions:
> + pci_disable_device(pdev);
> +err_pci_enable_dev:
> + dev_dbg(&pdev->dev, "pch_phub_probe returns %d\n", ret);
> + return ret;
> +}
> +
> +static void __devexit pch_phub_remove(struct pci_dev *pdev)
> +{
> +

drop blank line.

> + cdev_del(&pch_phub_dev);
> + dev_dbg(&pdev->dev,
> + "pch_phub_remove - cdev_del Invoked successfully\n");
> +
> + unregister_chrdev_region(pch_phub_dev_no, PCH_MINOR_NOS);
> + dev_dbg(&pdev->dev, "pch_phub_remove - "
> + "unregister_chrdev_region Invoked successfully\n");
> +
> + pci_unmap_rom(pdev, pch_phub_reg.pch_phub_extrom_base_address);
> +
> + pci_iounmap(pdev, pch_phub_reg.pch_phub_base_address);
> +
> + dev_dbg(&pdev->dev, "pch_phub_remove - "
> + "pci_iounmap Invoked successfully\n");
> +
> + pci_release_regions(pdev);
> + dev_dbg(&pdev->dev, "pch_phub_remove - "
> + "pci_release_regions Invoked successfully\n");
> +
> + pci_disable_device(pdev);
> + dev_dbg(&pdev->dev, "pch_phub_remove - "
> + "pci_disable_device Invoked successfully\n");
> +
> +}
> +
> +#ifdef CONFIG_PM

> +
> +static int pch_phub_resume(struct pci_dev *pdev)
> +{
> +

drop that blank line before the local data, but leave the one after local data.

> + int ret;
> +
> + pci_set_power_state(pdev, PCI_D0);
> + dev_dbg(&pdev->dev, "pch_phub_resume - "
> + "pci_set_power_state Invoked successfully\n");
> +
> + pci_restore_state(pdev);
> + dev_dbg(&pdev->dev, "pch_phub_resume - "
> + "pci_restore_state Invoked successfully\n");
> +
> + ret = pci_enable_device(pdev);
> + if (ret) {
> + dev_dbg(&pdev->dev,
> + "pch_phub_resume-pci_enable_device failed(ret=%d) ", ret);
> + return ret;
> + }
> +
> + dev_dbg(&pdev->dev, "pch_phub_resume - "
> + "pci_enable_device returns -%d\n", ret);

The '-' before %d could be confusing.

> +
> + pci_enable_wake(pdev, PCI_D3hot, 0);
> + dev_dbg(&pdev->dev, "pch_phub_resume - "
> + "pci_enable_wake Invoked successfully\n");
> +
> + pch_phub_restore_reg_conf(pdev);
> + dev_dbg(&pdev->dev, "pch_phub_resume - "
> + "pch_phub_restore_reg_conf Invoked successfully\n");
> +
> + pch_phub_reg.pch_phub_suspended = false;
> +
> + dev_dbg(&pdev->dev, "pch_phub_resume returns- %d\n", 0);

All of these dev_dbg() calls should use __func__ to get the function name:

dev_dbg(&pdev->dev, "%s returns: %d\n", __func__, 0);

(same for other functions)

(I dropped the '-' so that it won't look like a negative number.)

> + return 0;
> +}
> +#else
> +#define pch_phub_suspend NULL
> +#define pch_phub_resume NULL
> +#endif /* CONFIG_PM */
> +
> +static struct pci_device_id pch_phub_pcidev_id[] = {
> +
> + {PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_PCH1_PHUB)},
> + {0,}
> +};
> +
> +
> +static struct pci_driver pch_phub_driver = {
> + .name = "pch_phub",
> + .id_table = pch_phub_pcidev_id,
> + .probe = pch_phub_probe,
> + .remove = __devexit_p(pch_phub_remove),
> + .suspend = pch_phub_suspend,
> + .resume = pch_phub_resume
> +};
> +
> +/* pch_phub_pci_init - Implements the initialization functionality of
> + * the module.
> + */

drop obvious comments.

> +static int __init pch_phub_pci_init(void)
> +{
> + int ret;
> + ret = pci_register_driver(&pch_phub_driver);
> +
> + return ret;
> +}
> +
> +/* pch_phub_pci_exit - Implements the exit functionality of the module. */

drop obvious comment.

> +static void __exit pch_phub_pci_exit(void)
> +{
> + pci_unregister_driver(&pch_phub_driver);
> +

drop blank line.

> +}
> +
> +module_init(pch_phub_pci_init);
> +module_exit(pch_phub_pci_exit);
> +
> +MODULE_DESCRIPTION("PCH PACKET HUB PCI Driver");
> +MODULE_LICENSE("GPL");
> +

> diff --git a/drivers/char/pch_phub/pch_phub.h b/drivers/char/pch_phub/pch_phub.h
> new file mode 100755
> index 0000000..7baa272
> --- /dev/null
> +++ b/drivers/char/pch_phub/pch_phub.h
> @@ -0,0 +1,48 @@
> +#ifndef __PCH_PHUB_H__
> +#define __PCH_PHUB_H__
> +/*!

??

> + * Copyright (C) 2010 OKI SEMICONDUCTOR Co., LTD.
> + *
> + * 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.
> + */
> +
> +#define PHUB_IOCTL_MAGIC (0xf7)

ioctl numbers need to be added to Documentation/ioctl/ioctl-number.txt, please.

> +
> +/* Read GbE MAC address */
> +#define IOCTL_PHUB_READ_MAC_ADDR (_IOR(PHUB_IOCTL_MAGIC, 6, __u8[6]))
> +
> +/* Write GbE MAC address */
> +#define IOCTL_PHUB_WRITE_MAC_ADDR (_IOW(PHUB_IOCTL_MAGIC, 7, __u8[6]))
> +
> +/* SROM ACCESS Macro */
> +#define PCH_WORD_ADDR_MASK (~((1 << 2) - 1))
> +
> +/* Registers address offset */
> +#define PCH_PHUB_ID_REG 0x0000
> +#define PCH_PHUB_QUEUE_PRI_VAL_REG 0x0004
> +#define PCH_PHUB_RC_QUEUE_MAXSIZE_REG 0x0008
> +#define PCH_PHUB_BRI_QUEUE_MAXSIZE_REG 0x000C
> +#define PCH_PHUB_COMP_RESP_TIMEOUT_REG 0x0010
> +#define PCH_PHUB_BUS_SLAVE_CONTROL_REG 0x0014
> +#define PCH_PHUB_DEADLOCK_AVOID_TYPE_REG 0x0018
> +#define PCH_PHUB_INTPIN_REG_WPERMIT_REG0 0x0020
> +#define PCH_PHUB_INTPIN_REG_WPERMIT_REG1 0x0024
> +#define PCH_PHUB_INTPIN_REG_WPERMIT_REG2 0x0028
> +#define PCH_PHUB_INTPIN_REG_WPERMIT_REG3 0x002C
> +#define PCH_PHUB_INT_REDUCE_CONTROL_REG_BASE 0x0040
> +#define CLKCFG_REG_OFFSET 0x500
> +
> +#define PCH_PHUB_OROM_SIZE 15360
> +
> +#endif


--
~Randy
*** Remember to use Documentation/SubmitChecklist when testing your code ***
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/