* [PATCH v2] add mdio bus support to miitool
@ 2013-02-08 9:24 Sascha Hauer
2013-02-08 9:24 ` [PATCH 1/2] net phy: Add mdio buses to mdio_bus_type Sascha Hauer
2013-02-08 9:24 ` [PATCH 2/2] miitool: Add support for examing mdio bus Sascha Hauer
0 siblings, 2 replies; 7+ messages in thread
From: Sascha Hauer @ 2013-02-08 9:24 UTC (permalink / raw)
To: barebox
2nd version. This adds both the mdio buses and the phy devices
to the mdio_bus_type. This is in sync with Linux which also
adds the buses (for example i2c_adapter) and the devices (for example
i2c devices) to the same bus type. Linux uses a struct device_type
pointer in struct device to distinguish between both. As we do
not have this struct, we use the type_data field which has the
same purpose.
----------------------------------------------------------------
Sascha Hauer (2):
net phy: Add mdio buses to mdio_bus_type
miitool: Add support for examing mdio bus
commands/miitool.c | 94 +++++++++++++++++++++++++++++++++++---------
drivers/net/phy/mdio_bus.c | 18 +++++++++
drivers/net/phy/phy.c | 1 +
include/linux/phy.h | 7 ++++
4 files changed, 101 insertions(+), 19 deletions(-)
_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 1/2] net phy: Add mdio buses to mdio_bus_type
2013-02-08 9:24 [PATCH v2] add mdio bus support to miitool Sascha Hauer
@ 2013-02-08 9:24 ` Sascha Hauer
2013-02-08 9:24 ` [PATCH 2/2] miitool: Add support for examing mdio bus Sascha Hauer
1 sibling, 0 replies; 7+ messages in thread
From: Sascha Hauer @ 2013-02-08 9:24 UTC (permalink / raw)
To: barebox
Following the Linux example this adds both the mdio buses and the
phy devices found on the bus to the mdio_bus_type. This patch
uses the type_data field in struct device_d to distinguish between
both types.
Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
drivers/net/phy/mdio_bus.c | 18 ++++++++++++++++++
drivers/net/phy/phy.c | 1 +
include/linux/phy.h | 7 +++++++
3 files changed, 26 insertions(+)
diff --git a/drivers/net/phy/mdio_bus.c b/drivers/net/phy/mdio_bus.c
index d1d802b..173a3b2 100644
--- a/drivers/net/phy/mdio_bus.c
+++ b/drivers/net/phy/mdio_bus.c
@@ -47,6 +47,8 @@ int mdiobus_register(struct mii_bus *bus)
bus->dev.id = DEVICE_ID_DYNAMIC;
strcpy(bus->dev.name, "miibus");
bus->dev.parent = bus->parent;
+ bus->dev.bus = &mdio_bus_type;
+ bus->dev.type_data = (void *)MDIO_BUS_TYPE;
err = register_device(&bus->dev);
if (err) {
@@ -74,6 +76,22 @@ void mdiobus_unregister(struct mii_bus *bus)
}
EXPORT_SYMBOL(mdiobus_unregister);
+struct mii_bus *mdiobus_find(const char *name)
+{
+ struct device_d *dev;
+
+ for_each_mdiobus(dev) {
+ if (dev->type_data != (void *)MDIO_BUS_TYPE)
+ continue;
+
+ if (!strcmp(dev_name(dev), name))
+ return to_mii_bus(dev);
+ }
+
+ return NULL;
+}
+EXPORT_SYMBOL(mdiobus_find);
+
struct phy_device *mdiobus_scan(struct mii_bus *bus, int addr)
{
struct phy_device *phydev;
diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
index 1cf2cb9..33ce622 100644
--- a/drivers/net/phy/phy.c
+++ b/drivers/net/phy/phy.c
@@ -229,6 +229,7 @@ static int phy_register_device(struct phy_device* dev)
int ret;
dev->dev.parent = &dev->attached_dev->dev;
+ dev->dev.type_data = (void *)MDIO_PHY_DEVICE_TYPE;
ret = register_device(&dev->dev);
if (ret)
diff --git a/include/linux/phy.h b/include/linux/phy.h
index 6c9cac9..00820e8 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h
@@ -87,9 +87,13 @@ struct mii_bus {
};
#define to_mii_bus(d) container_of(d, struct mii_bus, dev)
+#define for_each_mdiobus(dev) \
+ bus_for_each_device(&mdio_bus_type, dev)
+
int mdiobus_register(struct mii_bus *bus);
void mdiobus_unregister(struct mii_bus *bus);
struct phy_device *mdiobus_scan(struct mii_bus *bus, int addr);
+struct mii_bus *mdiobus_find(const char *name);
/**
* mdiobus_read - Convenience function for reading a given MII mgmt register
@@ -286,5 +290,8 @@ int phy_register_fixup_for_uid(u32 phy_uid, u32 phy_uid_mask,
int (*run)(struct phy_device *));
int phy_scan_fixups(struct phy_device *phydev);
+#define MDIO_PHY_DEVICE_TYPE 1
+#define MDIO_BUS_TYPE 2
+
extern struct bus_type mdio_bus_type;
#endif /* __PHYDEV_H__ */
--
1.7.10.4
_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 2/2] miitool: Add support for examing mdio bus
2013-02-08 9:24 [PATCH v2] add mdio bus support to miitool Sascha Hauer
2013-02-08 9:24 ` [PATCH 1/2] net phy: Add mdio buses to mdio_bus_type Sascha Hauer
@ 2013-02-08 9:24 ` Sascha Hauer
2013-02-08 11:02 ` Alexander Aring
1 sibling, 1 reply; 7+ messages in thread
From: Sascha Hauer @ 2013-02-08 9:24 UTC (permalink / raw)
To: barebox
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;
}
+static struct mii_bus *mii_bus;
+static int mii_address;
+
+static u16 mdio_bus_read(int offset)
+{
+ return mdiobus_read(mii_bus, mii_address, offset);
+}
+
+static u16 (*mdio_read)(int offset);
+
/* Table of known MII's */
static const struct {
u_short id1, id2;
@@ -101,7 +114,7 @@ static char *media_list(int mask, int best)
return buf;
}
-static int show_basic_mii(int fd, int verbose)
+static int show_basic_mii(int verbose)
{
char buf[100];
int i, mii_val[32];
@@ -109,9 +122,9 @@ static int show_basic_mii(int fd, int verbose)
/* Some bits in the BMSR are latched, but we can't rely on being
the only reader, so only the current values are meaningful */
- mdio_read(fd, MII_BMSR);
+ mdio_read(MII_BMSR);
for (i = 0; i < ((verbose > 1) ? 32 : 8); i++)
- mii_val[i] = mdio_read(fd, i);
+ mii_val[i] = mdio_read(i);
if (mii_val[MII_BMCR] == 0xffff) {
fprintf(stderr, " No MII transceiver present!.\n");
@@ -213,18 +226,25 @@ static int show_basic_mii(int fd, int verbose)
static int do_miitool(int argc, char *argv[])
{
- char *filename;
+ char *filename = NULL;
+ char *devname = NULL;
int opt;
int argc_min;
- int fd;
int verbose;
+ int address = -1;
verbose = 0;
- while ((opt = getopt(argc, argv, "v")) > 0) {
+ while ((opt = getopt(argc, argv, "vd:a:")) > 0) {
switch (opt) {
case 'v':
verbose++;
break;
+ case 'd':
+ devname = optarg;
+ break;
+ case 'a':
+ address = simple_strtoul(optarg, NULL, 0);
+ break;
default:
return COMMAND_ERROR_USAGE;
}
@@ -232,27 +252,63 @@ static int do_miitool(int argc, char *argv[])
argc_min = optind + 1;
- if (argc < argc_min)
+ if (argc >= argc_min)
+ filename = argv[optind];
+
+ if (filename && devname) {
+ printf("both filename and devicename given\n");
return COMMAND_ERROR_USAGE;
+ }
- filename = argv[optind];
+ if (!filename && !devname) {
+ printf("no filename or devicename given\n");
+ return COMMAND_ERROR_USAGE;
+ }
+
+ if (filename) {
+ phy_fd = open(filename, O_RDONLY);
+ if (phy_fd < 0) {
+ printf("unable to read %s\n", filename);
+ return COMMAND_ERROR;
+ }
+
+ mdio_read = mdio_file_read;
+
+ show_basic_mii(verbose);
+ } else {
+ mii_bus = mdiobus_find(devname);
+
+ if (!mii_bus)
+ return -ENODEV;
- fd = open(filename, O_RDONLY);
- if (fd < 0) {
- printf("unable to read %s\n", filename);
- return COMMAND_ERROR;
+ mdio_read = mdio_bus_read;
+ if (address < 0) {
+ for (mii_address = 0; mii_address < PHY_MAX_ADDR; mii_address++) {
+ printf("`---- phyadr %d:\n",
+ mii_address);
+ show_basic_mii(verbose);
+ }
+ } else {
+ mii_address = address;
+ show_basic_mii(verbose);
+ }
}
- show_basic_mii(fd, verbose);
- close(fd);
+ if (filename)
+ close(phy_fd);
return COMMAND_SUCCESS;
}
BAREBOX_CMD_HELP_START(miitool)
-BAREBOX_CMD_HELP_USAGE("miitool [[[-v] -v] -v] <phy>\n")
-BAREBOX_CMD_HELP_SHORT("view status for MII <phy>.\n")
+BAREBOX_CMD_HELP_USAGE("miitool [OPTIONS] [phy]\n")
+BAREBOX_CMD_HELP_SHORT("view status mii phy device status\n")
+BAREBOX_CMD_HELP_SHORT("Use [phy] to view a phy connected to an ethernet device\n")
+BAREBOX_CMD_HELP_SHORT("or -d <devname> [-a <phyadr>] to examine a mii bus\n")
+BAREBOX_CMD_HELP_OPT("-v", "\tverbose, may be given multiple times\n")
+BAREBOX_CMD_HELP_OPT("-d <devname>", "work on miibus <devname>\n")
+BAREBOX_CMD_HELP_OPT("-a <phyadr>", "Use phy address <phyadr>, used together with -d\n")
BAREBOX_CMD_HELP_END
/**
--
1.7.10.4
_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 2/2] miitool: Add support for examing mdio bus
2013-02-08 9:24 ` [PATCH 2/2] miitool: Add support for examing mdio bus Sascha Hauer
@ 2013-02-08 11:02 ` Alexander Aring
2013-02-11 11:40 ` Sascha Hauer
0 siblings, 1 reply; 7+ messages in thread
From: Alexander Aring @ 2013-02-08 11:02 UTC (permalink / raw)
To: Sascha Hauer; +Cc: barebox
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);
Another thing:
You drop the errno in this function. I don't know about the driver bus
implementation if it can return -EIO or -EBUSY. I know it's not easy to
handle it because you return the buffer. But we can change the function
like this:
static int mdio_file_read(int offset, u16 *buf); to handle error.
It's only a small thing that I detected.
>
> +static struct mii_bus *mii_bus;
> +static int mii_address;
> +
> +static u16 mdio_bus_read(int offset)
> +{
> + return mdiobus_read(mii_bus, mii_address, offset);
> +}
> +
This would be:
static int mdio_bus_read(int offset, u16 *buf);
But it seems that this function will drop no errno.
> +static u16 (*mdio_read)(int offset);
static int (*mdio_read)(int offset, u16 *buf);
> +
> /* Table of known MII's */
> static const struct {
> u_short id1, id2;
> @@ -101,7 +114,7 @@ static char *media_list(int mask, int best)
> return buf;
> }
>
> -static int show_basic_mii(int fd, int verbose)
> +static int show_basic_mii(int verbose)
> {
> char buf[100];
> int i, mii_val[32];
In current implementation mii_val can be a u16 array.
> @@ -109,9 +122,9 @@ static int show_basic_mii(int fd, int verbose)
>
> /* Some bits in the BMSR are latched, but we can't rely on being
> the only reader, so only the current values are meaningful */
> - mdio_read(fd, MII_BMSR);
> + mdio_read(MII_BMSR);
Then we can handle it here.
> for (i = 0; i < ((verbose > 1) ? 32 : 8); i++)
> - mii_val[i] = mdio_read(fd, i);
> + mii_val[i] = mdio_read(i);
>
> if (mii_val[MII_BMCR] == 0xffff) {
> fprintf(stderr, " No MII transceiver present!.\n");
> @@ -213,18 +226,25 @@ static int show_basic_mii(int fd, int verbose)
>
> static int do_miitool(int argc, char *argv[])
> {
> - char *filename;
> + char *filename = NULL;
> + char *devname = NULL;
> int opt;
> int argc_min;
> - int fd;
> int verbose;
> + int address = -1;
>
> verbose = 0;
> - while ((opt = getopt(argc, argv, "v")) > 0) {
> + while ((opt = getopt(argc, argv, "vd:a:")) > 0) {
> switch (opt) {
> case 'v':
> verbose++;
> break;
> + case 'd':
> + devname = optarg;
> + break;
> + case 'a':
> + address = simple_strtoul(optarg, NULL, 0);
> + break;
> default:
> return COMMAND_ERROR_USAGE;
> }
> @@ -232,27 +252,63 @@ static int do_miitool(int argc, char *argv[])
>
> argc_min = optind + 1;
>
> - if (argc < argc_min)
> + if (argc >= argc_min)
> + filename = argv[optind];
> +
> + if (filename && devname) {
> + printf("both filename and devicename given\n");
> return COMMAND_ERROR_USAGE;
> + }
>
> - filename = argv[optind];
> + if (!filename && !devname) {
> + printf("no filename or devicename given\n");
> + return COMMAND_ERROR_USAGE;
> + }
> +
> + if (filename) {
> + phy_fd = open(filename, O_RDONLY);
> + if (phy_fd < 0) {
> + printf("unable to read %s\n", filename);
> + return COMMAND_ERROR;
> + }
> +
> + mdio_read = mdio_file_read;
> +
> + show_basic_mii(verbose);
If we handle error, don't forget to close phy_fd here.
Regards
Alex
> + } else {
> + mii_bus = mdiobus_find(devname);
> +
> + if (!mii_bus)
> + return -ENODEV;
>
> - fd = open(filename, O_RDONLY);
> - if (fd < 0) {
> - printf("unable to read %s\n", filename);
> - return COMMAND_ERROR;
> + mdio_read = mdio_bus_read;
> + if (address < 0) {
> + for (mii_address = 0; mii_address < PHY_MAX_ADDR; mii_address++) {
> + printf("`---- phyadr %d:\n",
> + mii_address);
> + show_basic_mii(verbose);
> + }
> + } else {
> + mii_address = address;
> + show_basic_mii(verbose);
> + }
> }
>
> - show_basic_mii(fd, verbose);
>
> - close(fd);
> + if (filename)
> + close(phy_fd);
>
> return COMMAND_SUCCESS;
> }
>
> BAREBOX_CMD_HELP_START(miitool)
> -BAREBOX_CMD_HELP_USAGE("miitool [[[-v] -v] -v] <phy>\n")
> -BAREBOX_CMD_HELP_SHORT("view status for MII <phy>.\n")
> +BAREBOX_CMD_HELP_USAGE("miitool [OPTIONS] [phy]\n")
> +BAREBOX_CMD_HELP_SHORT("view status mii phy device status\n")
> +BAREBOX_CMD_HELP_SHORT("Use [phy] to view a phy connected to an ethernet device\n")
> +BAREBOX_CMD_HELP_SHORT("or -d <devname> [-a <phyadr>] to examine a mii bus\n")
> +BAREBOX_CMD_HELP_OPT("-v", "\tverbose, may be given multiple times\n")
> +BAREBOX_CMD_HELP_OPT("-d <devname>", "work on miibus <devname>\n")
> +BAREBOX_CMD_HELP_OPT("-a <phyadr>", "Use phy address <phyadr>, used together with -d\n")
> BAREBOX_CMD_HELP_END
>
> /**
> --
> 1.7.10.4
>
>
> _______________________________________________
> barebox mailing list
> barebox@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/barebox
_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 2/2] miitool: Add support for examing mdio bus
2013-02-08 11:02 ` Alexander Aring
@ 2013-02-11 11:40 ` Sascha Hauer
2013-02-11 23:13 ` Alexander Aring
0 siblings, 1 reply; 7+ messages in thread
From: Sascha Hauer @ 2013-02-11 11:40 UTC (permalink / raw)
To: Alexander Aring; +Cc: barebox
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?
>
>
> Another thing:
> You drop the errno in this function. I don't know about the driver bus
> implementation if it can return -EIO or -EBUSY. I know it's not easy to
> handle it because you return the buffer. But we can change the function
> like this:
>
> static int mdio_file_read(int offset, u16 *buf); to handle error.
>
> It's only a small thing that I detected.
Ok, can fix this.
Sascha
--
Pengutronix e.K. | |
Industrial Linux Solutions | http://www.pengutronix.de/ |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 2/2] miitool: Add support for examing mdio bus
2013-02-11 11:40 ` Sascha Hauer
@ 2013-02-11 23:13 ` Alexander Aring
2013-02-12 7:35 ` Sascha Hauer
0 siblings, 1 reply; 7+ messages in thread
From: Alexander Aring @ 2013-02-11 23:13 UTC (permalink / raw)
To: Sascha Hauer; +Cc: barebox
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
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 2/2] miitool: Add support for examing mdio bus
2013-02-11 23:13 ` Alexander Aring
@ 2013-02-12 7:35 ` Sascha Hauer
0 siblings, 0 replies; 7+ messages in thread
From: Sascha Hauer @ 2013-02-12 7:35 UTC (permalink / raw)
To: Alexander Aring; +Cc: barebox
On Tue, Feb 12, 2013 at 12:13:06AM +0100, Alexander Aring wrote:
> 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.
I also vote for option 2; option 1 could be implemented should the need
arise, but I currently don't see it.
Sascha
--
Pengutronix e.K. | |
Industrial Linux Solutions | http://www.pengutronix.de/ |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2013-02-12 7:36 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-02-08 9:24 [PATCH v2] add mdio bus support to miitool Sascha Hauer
2013-02-08 9:24 ` [PATCH 1/2] net phy: Add mdio buses to mdio_bus_type Sascha Hauer
2013-02-08 9:24 ` [PATCH 2/2] miitool: Add support for examing mdio bus Sascha Hauer
2013-02-08 11:02 ` Alexander Aring
2013-02-11 11:40 ` Sascha Hauer
2013-02-11 23:13 ` Alexander Aring
2013-02-12 7:35 ` Sascha Hauer
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox