[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]
Re: HD6446x driver
On Fri, May 07, 2010 at 22:53:46 +0900, KIYOHARA Takashi wrote:
> I knew UART of HD64461 did not work on PERSONA recently. This trouble
> will be corrected by me.
I guess the COM_REGMAP part of the patch does that. Do AFE clocks
need to be enabled too?
> Moreover, because I look alike HD64461 and HD64465 very much, it proposes
> to merge these.
> # hd64461if + hd64465if => shc (SH companion)
> Do you have a big problem in merging these? Also more better name?
It's been quite a long time since I looked at hd6446x manuals in any
details, so I don't remember how similar they are. If we can really
merge them, I guess that's good. OTOH, I wonder if the return of
investment is really that promising.
> Index: conf/GENERIC
> RCS file: /cvsroot/src/sys/arch/hpcsh/conf/GENERIC,v
> retrieving revision 1.87
> diff -u -r1.87 GENERIC
> --- conf/GENERIC 5 Dec 2009 20:11:14 -0000 1.87
> +++ conf/GENERIC 27 Apr 2010 03:07:39 -0000
> @@ -252,6 +252,7 @@
> # HITACHI PERSONA (HPW-50PAD, HPQ-650PA)
> com0 at hd64461if?
> +options COM_REGMAP
Shouldn't this be in std.hpcsh instead of the specific kernel config?
> @@ -107,11 +104,18 @@
> hd64461uartcninit(struct consdev *cp)
> + struct com_regs regs;
> + int i;
> - comcnattach(hd64461uart_chip.io_tag, 0x0, COMCN_SPEED, COM_FREQ,
> - COM_TYPE_NORMAL, CONMODE);
> + memset(®s, 0, sizeof(regs));
> + regs.cr_iot = hd64461uart_chip.io_tag;
> + regs.cr_iobase = 0x00000000;
> + regs.cr_nports = COM_NPORTS;
> + for (i = 0; i < __arraycount(regs.cr_map); i++)
> + regs.cr_map[i] = com_std_map[i] << 1;
> + comcnattach1(®s, COMCN_SPEED, COM_FREQ, COM_TYPE_NORMAL, CONMODE);
> hd64461uart_chip.console = 1;
May be define a macro for this, like COM_INIT_REGS, but for the map we
use here. You are doing the same initialization several times in this
> if (strcmp(kgdb_devname, "hd64461uart") != 0)
> - return (1);
> + return 1;
It's better not to mix KNF changes with real changes.
> @@ -188,7 +203,7 @@
> hd6446x_intr_establish(HD64461_INTC_UART, IST_LEVEL, IPL_TTY,
> - comintr, self);
> + comintr, csc);
This looks like the real bug (fallout from splitting softc) you are
fixing. In that case it's better to make it a separate commit.
uwe%stderr.spb.ru@localhost | Zu Grunde kommen
http://snark.ptc.spbu.ru/~uwe/ | Ist zu Grunde gehen
Main Index |
Thread Index |