Hi Greg, This patch has been accepted by the upstream. http://git.zen-kernel.org/?p=kernel/mmotm.git;a=commit;h=79a0ac666c36d75c98039251cbe0775ea54d9c3e
Arjan requires us to send this patch again to Meego-Dev. Hi Okada-san, Would you please answer the question of Greg? Best Regards, Qi. -----Original Message----- From: [email protected] [mailto:[email protected]] On Behalf Of Greg KH Sent: Tuesday, August 03, 2010 11:57 AM To: Development for the MeeGo Project (discussion list) Cc: Khor, Andrew Chih Howe Subject: Re: [MeeGo-dev] [MeeGo-Dev][PATCH] Topcliff: Update PCH_PHUB driver to 2.6.35 On Tue, Aug 03, 2010 at 11:34:32AM +0800, Wang, Qi 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 has 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. > The driver creates a character device /dev/pch_phub. That device file > supports the following operations: > > read() :Read Option ROM data of SROM > write():Write Option ROM data of SROM Shouldn't that just be done with a standard sysfs binary file instead of a character device node? > ioctl():Read/Write MAC address of SROM What is the MAC address for? Why would you want to change it? Again, a sysfs file instead? > + dev_dbg(&pdev->dev, "%s : " You forgot to terminate the line with a \n character. > +/** > + * pch_phub_write_gbe_mac_addr() - Write MAC address > + * @offset_address: Gigabit Ethernet MAC address offset value. > + * @data: Gigabit Ethernet MAC address value. > + */ Ah, it's a network MAC address. Then why not just allow the "standard" interface that Linux provides for changing the MAC address of a network device, and not worry about the MAC address here in the hub controller at all? > +/* 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 All of these do not need to be in the .h file, right? Even if you were to keep the ioctl/read/write mess. Please just move this to sysfs and not use a character device. Also, why post this here and not on the linux-kernel mailing list? thanks, greg k-h _______________________________________________ MeeGo-dev mailing list [email protected] http://lists.meego.com/listinfo/meego-dev _______________________________________________ MeeGo-dev mailing list [email protected] http://lists.meego.com/listinfo/meego-dev
