Hi Ferruh,

I have incorporated most of your comments in V3 of this series. I'll send the 
series by today.
For those, which will not be included in this version, I have added some 
comments, please see inline.

> <...>
> 
> > +
> > +/* for link status and IOCTL support using pfe character device
> > + * XXX: Should be kept in sync with Kernel module
> 
> How can we know if it is in sync with kernel module, is there any check in the
> code?
There is no check for this in the code, but it is mandatory to load "pfe.ko"  
kernel
Module to run PFFE driver, I have added this requirement in document under 
prerequisites section.
 
<...>
> 
> > +unsigned int pfe_svr = SVR_LS1012A_REV1;
> 
> There are some checks for this in hal layer, should this be a runtime option
> instead of hardcoded in the .c file?
I agree with you, It should be a runtime option. Is it ok, if I send a separate 
patch
for this? It may take some time as I have to look into the manual to find 
possible ways
to get svr value from the HW. I'll add a comment here for this.

<...>
> > +   g_pfe->ddr_size = ddr_size;
> > +
> > +   fd = open("/dev/mem", O_RDWR);
> > +   g_pfe->cbus_baseaddr = mmap(NULL, cbus_size, PROT_READ |
> PROT_WRITE,
> > +                                   MAP_SHARED, fd, cbus_addr);
> 
> Above reads device information from a device tree file and creates a new
> mapping
> for its address space. I wonder if device can be probbed as we do the pcie
> devices, do you have to depend on the 'of', can't the driver registered for a
> pysical device?
> 
These eth devices are actual Virtual interfaces created by Client driver (a part
of the ppfe driver).  Actually, HW provides a single interface (Host 
interface), which
is the only device accessible by the userspace driver for enqueue and dequeue 
and ppfe driver create
two virtual interfaces out of this single host interface. Also, this device is 
not on pcie bus.

Thanks,
Gagan 

Reply via email to