Re: [PATCH 2/2] miitool: Add support for examing mdio bus

[Alexander Aring <alex.aring@gmail.com>]

Hi Sascha,

On Mon, Feb 11, 2013 at 12:40:23PM +0100, Sascha Hauer wrote:
> On Fri, Feb 08, 2013 at 12:02:20PM +0100, Alexander Aring wrote:
> > Hi Sascha,
> > 
> > On Fri, Feb 08, 2013 at 10:24:17AM +0100, Sascha Hauer wrote:
> > > Current miitool usage is limited to a single phy on a mdio bus.
> > > 
> > > For debugging purposes it is useful to be able to examine a mdio bus
> > > instead of a single phy only.
> > > 
> > > This adds support for this to the miitool command.
> > > 
> > > Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> > > ---
> > >  commands/miitool.c |   94 +++++++++++++++++++++++++++++++++++++++++-----------
> > >  1 file changed, 75 insertions(+), 19 deletions(-)
> > > 
> > > diff --git a/commands/miitool.c b/commands/miitool.c
> > > index 3a9ac45..b9e2333 100644
> > > --- a/commands/miitool.c
> > > +++ b/commands/miitool.c
> > > @@ -37,23 +37,36 @@
> > >  #include <linux/stat.h>
> > >  #include <xfuncs.h>
> > >  #include <linux/mii.h>
> > > +#include <linux/phy.h>
> > >  
> > > -static u16 mdio_read(int fd, int offset)
> > > +static int phy_fd;
> > > +
> > > +static u16 mdio_file_read(int offset)
> > >  {
> > >  	int ret;
> > >  	u16 buf;
> > >  
> > > -	ret = lseek(fd, offset << 1, SEEK_SET);
> > > +	ret = lseek(phy_fd, offset << 1, SEEK_SET);
> > >  	if (ret < 0)
> > >  		return 0;
> > >  
> > > -	ret = read(fd, &buf, sizeof(u16));
> > > +	ret = read(phy_fd, &buf, sizeof(u16));
> > >  	if (ret < 0)
> > >  		return 0;
> > >  
> > >  	return buf;
> > >  }
> > 
> > I made a similar function in nandtest and I am thinking about to add a
> > pread and pwrite into file operations.
> > Is it welcome to send patches for adding pwrite and pread into file
> > operations? I would implement it like this but with a prototype
> > which looks like:
> > 
> > ssize_t pread(int fildes, void *buf, size_t nbyte, off_t offset);
> 
> Sounds good. Just to be sure: With 'file operations' you do not mean
> struct file_operations, right?
> 

I can do this in two ways.

Option 1:

Write 'file_operations helper functions' for pwrite and pread. Each driver can
assign this to pwrite and pread callbacks in file_operations struct.

Similar we can do that for a normal file fs_driver_d.

This helper functions just call lseek and write or read for specific
device/filesystem.
If callback is null, driver or filesystem doesn't support it.

pread and pwrite implementation in fs/fs.c will call assign callback
from fs_driver_d.

Option 2:

Just add a pwrite and pread function in fs/fs.c which calls for a
filedescriptor lseek and then write or read.

---

I prefer option 2. This implementation used smaller size and it works
like option 1. It's just a call of read or write with an specific offset.

But in option 1 a driver or filesystem can replace this function with an
another function which can do something other than just call lseek and then
write or read.

Regards
Alex

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox