From: Jiri Slaby <jirislaby@gmail.com>
To: "Øyvind Aabling" <Oyvind.Aabling@uni-c.dk>
Cc: linux-kernel@vger.kernel.org
Subject: Re: [PATCH] drivers/char/moxa.c, kernel 2.6.23.14
Date: Mon, 21 Jan 2008 23:51:35 +0100 [thread overview]
Message-ID: <479521F7.9050701@gmail.com> (raw)
In-Reply-To: <Pine.LNX.4.64.0801202145230.18101@dbs1.uni-c.dtu.dk>
On 01/20/2008 10:45 PM, Øyvind Aabling wrote:
> moxa.c changes to MOXA_GET_CONF ioctl breaks moxaload (userspace
> application for firmware download to MOXA Intellio CPU boards).
>
> Attached (and included inline below) is a patch to solve a problem
> introduced by changes to struct moxa_board_conf in drivers/char/moxa.c.
>
> AFAICS from the changelogs, moxa.c was rewritten to a new API in 2.6.21,
> but I've only tested it (and moxaload) on kernels up to 2.6.19.2
> (where it works) and on 2.6.22.6 and various later kernels,
> including the latest (2.6.23.14), where it doesn't work.
>
> Steps to reproduce:
>
> Call the moxaload program (from MOXA) to download the firmware.
Thanks for pointing this out.
> moxaload will fail on most systems (all the ones I've tried), because it
> thinks there is a memory conflict, although this behaviour will depend
> on the exact contents of struct moxa_board_conf (in drivers/char/moxa.c).
>
> The problem is, that moxaload uses the MOXA_GET_CONF ioctl,
> which returns (verbatim) the contents of struct moxa_board_conf,
> the structure (and contents) of which has changed heavily.
>
> This patch corrects this problem by reverting the behaviour of the
> MOXA_GET_CONF ioctl, so it returns the info that moxaload expects.
>
> I'm not on the kernel list, so please CC:
> me with any questions and/or comments.
>
> To Jiri Slaby <jirislaby@gmail.com>:
>
> I've CC'ed this to you, although linux/MAINTAINERS doesn't
> mention you as the maintainer of moxa.c, since the changelogs
> seems to indicate, that you're the current maintainer.
>
> linux/MAINTAINERS mentions you (Jiri) as the maintainer
> of mxser, but that is the driver for other MOXA
> boards, so I hope that I've guessed right ...
Yeah, at least I know what's the problem about :).
> --- linux-2.6.23.14/drivers/char/moxa.c 2008-01-14 21:49:56.000000000
> +0100
> +++ linux/drivers/char/moxa.c 2008-01-20 18:30:15.000000000 +0100
> @@ -109,6 +109,8 @@
> int busType;
>
> int loadstat;
> + unsigned short busNum;
> + unsigned short devNum;
>
> void __iomem *basemem;
> void __iomem *intNdx;
> @@ -116,6 +118,16 @@
> void __iomem *intTable;
> } moxa_boards[MAX_BOARDS];
>
> +/* Used by userspace application moxaload (firmware download) */
> +static struct moxa_board_info {
> + int boardType;
> + int numPorts;
> + unsigned long baseAddr;
> + int busType;
> + unsigned short busNum;
> + unsigned short devNum;
> +} moxa_board_info[MAX_BOARDS];
> +
Hrm, too ugly (sadly as same as it was).
- not 32/64 bit compatible due to the ulong there.
- not exported to the world via include/linux (the reason why I removed it, they
use something, which they know nothing about).
- I would rather see true firmware loading.
Would you be willing to test such a patch for point no. 3?
> struct mxser_mstatus {
> tcflag_t cflag;
> int cts;
> @@ -304,6 +316,9 @@
> goto err;
>
> board->boardType = board_type;
> + board->baseAddr = pci_resource_start(pdev, 2);
you missed isa cards...
> + board->busNum = pdev->bus->number;
> + board->devNum = PCI_SLOT(pdev->devfn);
> switch (board_type) {
> case MOXA_BOARD_C218_ISA:
> case MOXA_BOARD_C218_PCI:
> @@ -1494,8 +1509,16 @@
> }
> switch (cmd) {
> case MOXA_GET_CONF:
> - if(copy_to_user(argp, &moxa_boards, MAX_BOARDS *
> - sizeof(struct moxa_board_conf)))
> + for (i = 0; i < MAX_BOARDS; i++) {
> + moxa_board_info[i].boardType = moxa_boards[i].boardType;
> + moxa_board_info[i].numPorts = moxa_boards[i].numPorts;
> + moxa_board_info[i].baseAddr = moxa_boards[i].baseAddr;
> + moxa_board_info[i].busType = moxa_boards[i].busType;
> + moxa_board_info[i].busNum = moxa_boards[i].busNum;
> + moxa_board_info[i].devNum = moxa_boards[i].devNum;
> + }
> + if(copy_to_user(argp, &moxa_board_info, MAX_BOARDS *
> + sizeof(struct moxa_board_info)))
> return -EFAULT;
> return (0);
> case MOXA_INIT_DRIVER:
thanks,
--js
next parent reply other threads:[~2008-01-21 22:51 UTC|newest]
Thread overview: 17+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <Pine.LNX.4.64.0801202145230.18101@dbs1.uni-c.dtu.dk>
2008-01-21 22:51 ` Jiri Slaby [this message]
2008-01-22 10:23 ` [PATCH] drivers/char/moxa.c, kernel 2.6.23.14 Oyvind Aabling
2008-01-23 15:37 ` Jiri Slaby
2008-01-24 23:34 ` Oyvind Aabling
2008-01-24 23:38 ` Jiri Slaby
2008-01-24 9:32 ` [RFC 1/5] Char: moxa, remove static isa support Jiri Slaby
2008-01-24 9:32 ` [RFC 2/5] Char: moxa, cleanup module-param passed isa init Jiri Slaby
2008-01-24 9:32 ` [RFC 3/5] Char: moxa, pci io space fixup Jiri Slaby
2008-01-24 9:32 ` [RFC 4/5] Char: moxa, fix TIOC(G/S)SOFTCAR param Jiri Slaby
2008-01-24 9:32 ` [RFC 5/5] Char: moxa, add firmware loading Jiri Slaby
2008-01-27 19:16 ` [RFC 1/6] " Jiri Slaby
2008-01-27 19:16 ` [RFC 2/6] Char: moxa, merge c2xx and c320 " Jiri Slaby
2008-01-27 19:16 ` [RFC 3/6] Char: moxa, remove port->port Jiri Slaby
2008-01-27 19:16 ` [RFC 4/6] Char: moxa, remove unused port entries Jiri Slaby
2008-01-27 19:16 ` [RFC 5/6] Char: moxa, centralize board readiness Jiri Slaby
2008-01-27 19:16 ` [RFC 6/6] Char: moxa, timer cleanup Jiri Slaby
2008-01-21 12:35 [PATCH] drivers/char/moxa.c, kernel 2.6.23.14 Oyvind Aabling
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=479521F7.9050701@gmail.com \
--to=jirislaby@gmail.com \
--cc=Oyvind.Aabling@uni-c.dk \
--cc=linux-kernel@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).