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@xxxxxxxxxxxxxx>
> ---
> 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@xxxxxxxxxxxxxxxxxxx
> http://lists.infradead.org/mailman/listinfo/barebox
_______________________________________________
barebox mailing list
barebox@xxxxxxxxxxxxxxxxxxx
http://lists.infradead.org/mailman/listinfo/barebox
