Hi Ohtake, On Tue, 2010-08-03 at 12:48 +0200, ext Masayuki Ohtake wrote: > Hi Greg KH, > > I have added my answer below. > Please find '<MASA>'. > > Thanks, Ohtake > > ----- Original Message ----- > From: "Greg KH" <gre...@suse.de> > To: "Development for the MeeGo Project (discussion list)" > <meego-dev@meego.com> > Cc: "Khor, Andrew Chih Howe" <andrew.chih.howe.k...@intel.com> > Sent: Tuesday, August 03, 2010 12:57 PM > 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? > <MASA>This driver also sets Phub hw configuration. > Thus, phub driver should be integrated as character device. > > > > > > 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? > <MASA> The MAC address is used by GbE. > > > > > > > + dev_dbg(&pdev->dev, "%s : " > > > > You forgot to terminate the line with a \n character. > <MASA> The above debug message is one sentence. Thus, we didn't add \n > character.
Terminating "\n" will help in starting the next debug message on new line. > > > > > +/** > > > + * 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? > <MASA> Please refer above answer. > > > > > > +/* 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. > <MASA> Please refer above answer. > > > > > Also, why post this here and not on the linux-kernel mailing list? > <MASA>This patch have already accepted by linux kernel upstream. Is it in Linus's tree or Andrew Morton's -mm tree? Cheers, Ameya. _______________________________________________ MeeGo-dev mailing list MeeGo-dev@meego.com http://lists.meego.com/listinfo/meego-dev
