Re: btuart and SOCKET Bluetooth CF
Hi! Iain,
From: Iain Hibbert
Date: Wed, 17 Feb 2010 10:12:28 + (GMT)
> On Tue, 16 Feb 2010, KIYOHARA Takashi wrote:
>
> /* sc_state */
> -#define BTUART_RECV_PKT_TYPE 0 /* packet type */
> +enum state {
> + BTUART_RECV_PKT_TYPE, /* packet type */
>
> please don't make this unrelated change. I don't like it because sc_state
> should really become "enum state" rather than int but then you need to add
> forward declarations..
It relates.
I have increased the value for DTL. However, this value doesn't have
the meaning. And, this is not used to calculate. Then, I think that
the enum that should not consider the value is more convenient. There
is danger of defining the same value for the macro.
> (btw, no comma is permitted on final enum values)
This is because of convenient for the addition in the future. The diff
becomes smaller. ;-)
> --- btattach.c6 Dec 2009 12:55:46 - 1.5
> +++ btattach.c16 Feb 2010 03:28:53 -
> @@ -54,13 +54,6 @@
:
> This is unrelated, and I did it just now..
Thanks.
> +static int
> +btuart_dtl_probe(dev_t dev)
> +{
> +#if NCOM > 0
:
> +#endif
> + return 0;
> +}
>
> I really dislike this function, it depends on internals of com driver [btw
> COMUNIT(x) would be (minor(x) & COMUNIT_MASK)] and pcmcia and using this
> method prevents a soft emulator and/or needs kernel modifications to
> handle any new device.
We are expecting the minor number to be obtained. In my opinion, it is
necessary to open() the device node for that. Therefore, only minor()
is called. It thinks dependence on the structure of pcmcia(4) for other
way not to exist though it is very regrettable. ;-<
Thanks,
--
kiyohara
Index: btuart.c
===
RCS file: /cvsroot/src/sys/dev/bluetooth/btuart.c,v
retrieving revision 1.24
diff -u -r1.24 btuart.c
--- btuart.c18 Feb 2010 07:24:16 - 1.24
+++ btuart.c18 Feb 2010 07:30:01 -
@@ -1,7 +1,7 @@
/* $NetBSD: btuart.c,v 1.24 2010/02/18 07:24:16 kiyohara Exp $ */
/*-
- * Copyright (c) 2006, 2007 KIYOHARA Takashi
+ * Copyright (c) 2006, 2007, 2010 KIYOHARA Takashi
* All rights reserved.
*
* Redistribution and use in source and binary forms, with or without
@@ -29,6 +29,8 @@
#include
__KERNEL_RCSID(0, "$NetBSD: btuart.c,v 1.24 2010/02/18 07:24:16 kiyohara Exp
$");
+#include "com.h"
+
#include
#include
#include
@@ -49,8 +51,14 @@
#include
#include
+#if NCOM > 0
+#include
+#include
+#endif
+
#include "ioconf.h"
+
struct btuart_softc {
device_tsc_dev;
struct tty *sc_tp; /* tty pointer */
@@ -61,6 +69,7 @@
int sc_state; /* receive state */
int sc_want;/* how much we want */
+ boolsc_pad; /* pad will receive before next pkt */
struct mbuf * sc_rxp; /* incoming packet */
boolsc_xmit;/* transmit is active */
@@ -73,13 +82,25 @@
};
/* sc_state */
-#define BTUART_RECV_PKT_TYPE 0 /* packet type */
-#define BTUART_RECV_ACL_HDR1 /* acl header */
-#define BTUART_RECV_SCO_HDR2 /* sco header */
-#define BTUART_RECV_EVENT_HDR 3 /* event header */
-#define BTUART_RECV_ACL_DATA 4 /* acl packet data */
-#define BTUART_RECV_SCO_DATA 5 /* sco packet data */
-#define BTUART_RECV_EVENT_DATA 6 /* event packet data */
+#define BTUART_RECV_PKT_TYPE 0 /* packet type */
+#define BTUART_RECV_ACL_HDR1 /* acl header */
+#define BTUART_RECV_SCO_HDR2 /* sco header */
+#define BTUART_RECV_EVENT_HDR 3 /* event header */
+#define BTUART_RECV_ACL_DATA 4 /* acl packet data */
+#define BTUART_RECV_SCO_DATA 5 /* sco packet data */
+#define BTUART_RECV_EVENT_DATA 6 /* event packet data */
+#define BTUART_RECV_DTL_HDR7 /* DTL header */
+#define BTUART_RECV_DTL_CTRL_DATA 8 /* DTL control data */
+#define BTUART_RECV_DTL_ACL_DATA 9 /* DTL acl data */
+#define BTUART_RECV_DTL_SCO_DATA 10 /* DTL sco data */
+#define BTUART_RECV_DTL_EVENT_DATA 11 /* DTL event data */
+
+struct btuart_dtl_header { /* NOKIA DTL-1/4 header */
+ uint8_t type; /* packet type */
+ uint8_t rsvd;
+ uint16_t len; /* data length */
+} __packed;
+#define BTUART_DTL_HEADER_TYPE 0x80
void btuartattach(int);
static int btuart_match(device_t, cfdata_t, void *);
@@ -99,12 +120,20 @@
static void btuart_output_sco(device_t, struct mbuf *);
static void btuart_stats(device_t, struct bt_stats *, int);
+static int btuart_dtl_probe(dev_t);
+static void btuart_dtl_attach(device_t, device_t, void *);
+static void btuart_dtl_output_cmd(device_t, struct mbuf *);
+static void btuart_dtl_output_acl(device_t, struct mbuf
Re: wbsio(4): Winbond Super I/O driver for lm(4) over wbsio(4) attachment
On Wed, 17 Feb 2010, Constantine Aleksandrovich Murenin wrote: 1. Does it make sense to extend this to any other functions of the super-IO chip? Are any others likely to ever show up on non- standard ISA ports? Yes, wbsio(4) could later provide Watchdog Timer support, but such timer is accessed directly through the Super I/O ports, not through a separate set of ports as is the case with Hardware Monitor. OK. 2. In the man page, please change the statement +driver provides support for the Winbond LPC Super I/O ICs. to +driver provides configuration support for the Winbond LPC +Super I/O ICs. Interesting point, but perhaps this is being too specific? Specifically since it could later even support the watchdog timer? :-) OK. 3. As written, wbsio_print() assumes that the ir_size of the lm(4)'s isa_attach_args is the same as the size of the wbsio(4)'s ir_size. Is this really correct? Or should we explicitly set the ir_size member before calling config_found()? I don't see such assumption anywhere in wbsio(4). In any case, ir_size is always set in the match procedure of the respective matching driver, e.g. it is still set to 8 for lm(4) in lm_isa_match(): wbsio0 at isa0 port 0x2e-0x2f: Winbond LPC Super I/O W83627DHG rev 0x23 lm0 at wbsio0 port 0x290-0x297 lm0: Using default temp sensors lm0: Winbond W83627DHG Hardware monitor OK, I was not aware that each device's *_match() routine would set the ir_size. Thanks for clarifying. Curiosity: I will confess that I'm writing this section without datasheets in front of me... But I notice that there are some differences in the lists of devices between wbsio(4) and lm(4). A few IDs are the same, but most are different. I also notice that wbsio(4) uses the device-ID, while lm(4) uses the chip-ID. Question: 4. So, would it make sense to be consistent and use the same xxx-ID in both drivers? (In which case, wbsio(4) could simply include nslm7xvar.h rather than having its own ID definitions.) Your observation is correct- the namespace for IDs is different, so it would not make much sense to unify this with nslm7xvar.h in any way. OK Question: 5. And, does the list of devices in wbsio(4) deliberately exclude the rest of the lm(4) devices? Or could some/all of the remaining chips from lm(4) be added to wbsio(4)? I think all supported devices were actually already included, and since wbsio(4) was written, no new devices were introduced to lm(4). Obviously, not all of those supported by lm(4) could ever show up on the isa bus - for example, W83792D and a few others are SMBus-only. OK. You've addressed all my concerns. Let's give a couple days for others to comment. - | Paul Goyette | PGP DSS Key fingerprint: | E-mail addresses: | | Customer Service | FA29 0E3B 35AF E8AE 6651 | paul at whooppee.com | | Network Engineer | 0786 F758 55DE 53BA 7731 | pgoyette at juniper.net | | Kernel Developer | | pgoyette at netbsd.org | -
Re: wbsio(4): Winbond Super I/O driver for lm(4) over wbsio(4) attachment
On 17 February 2010 07:57, Paul Goyette wrote: > On Tue, 16 Feb 2010, Constantine Aleksandrovich Murenin wrote: > >> Hi, >> >> The wbsio(4) driver allows one to attach lm(4) on non-standard IO ports, >> or, alternatively, discover the said non-standard ports, so that an ap- >> propriate lm0 configuration line could be devised for the configuration >> file of the kernel. >> >> Best regards, >> Constantine. > > Kewl... > > Question: > > 1. Does it make sense to extend this to any other functions of the > super-IO chip? Are any others likely to ever show up on non- > standard ISA ports? Yes, wbsio(4) could later provide Watchdog Timer support, but such timer is accessed directly through the Super I/O ports, not through a separate set of ports as is the case with Hardware Monitor. > Suggestion: > 2. In the man page, please change the statement > > +driver provides support for the Winbond LPC Super I/O ICs. > > to > > +driver provides configuration support for the Winbond LPC > +Super I/O ICs. Interesting point, but perhaps this is being too specific? Specifically since it could later even support the watchdog timer? :-) > Question: > 3. As written, wbsio_print() assumes that the ir_size of the lm(4)'s > isa_attach_args is the same as the size of the wbsio(4)'s ir_size. > Is this really correct? Or should we explicitly set the ir_size > member before calling config_found()? I don't see such assumption anywhere in wbsio(4). In any case, ir_size is always set in the match procedure of the respective matching driver, e.g. it is still set to 8 for lm(4) in lm_isa_match(): wbsio0 at isa0 port 0x2e-0x2f: Winbond LPC Super I/O W83627DHG rev 0x23 lm0 at wbsio0 port 0x290-0x297 lm0: Using default temp sensors lm0: Winbond W83627DHG Hardware monitor > Curiosity: > I will confess that I'm writing this section without datasheets in > front of me... But I notice that there are some differences in the > lists of devices between wbsio(4) and lm(4). A few IDs are the same, > but most are different. > > I also notice that wbsio(4) uses the device-ID, while lm(4) uses the > chip-ID. > > Question: > 4. So, would it make sense to be consistent and use the same xxx-ID in > both drivers? (In which case, wbsio(4) could simply include > nslm7xvar.h rather than having its own ID definitions.) Your observation is correct — the namespace for IDs is different, so it would not make much sense to unify this with nslm7xvar.h in any way. > Question: > 5. And, does the list of devices in wbsio(4) deliberately exclude the > rest of the lm(4) devices? Or could some/all of the remaining > chips from lm(4) be added to wbsio(4)? I think all supported devices were actually already included, and since wbsio(4) was written, no new devices were introduced to lm(4). Obviously, not all of those supported by lm(4) could ever show up on the isa bus — for example, W83792D and a few others are SMBus-only. Cheers, Constantine.SU.
Re: wbsio(4): Winbond Super I/O driver for lm(4) over wbsio(4) attachment
On Tue, 16 Feb 2010, Constantine Aleksandrovich Murenin wrote: Hi, The wbsio(4) driver allows one to attach lm(4) on non-standard IO ports, or, alternatively, discover the said non-standard ports, so that an ap- propriate lm0 configuration line could be devised for the configuration file of the kernel. Best regards, Constantine. Kewl... Question: 1. Does it make sense to extend this to any other functions of the super-IO chip? Are any others likely to ever show up on non- standard ISA ports? Suggestion: 2. In the man page, please change the statement +driver provides support for the Winbond LPC Super I/O ICs. to +driver provides configuration support for the Winbond LPC +Super I/O ICs. Question: 3. As written, wbsio_print() assumes that the ir_size of the lm(4)'s isa_attach_args is the same as the size of the wbsio(4)'s ir_size. Is this really correct? Or should we explicitly set the ir_size member before calling config_found()? Curiosity: I will confess that I'm writing this section without datasheets in front of me... But I notice that there are some differences in the lists of devices between wbsio(4) and lm(4). A few IDs are the same, but most are different. I also notice that wbsio(4) uses the device-ID, while lm(4) uses the chip-ID. Question: 4. So, would it make sense to be consistent and use the same xxx-ID in both drivers? (In which case, wbsio(4) could simply include nslm7xvar.h rather than having its own ID definitions.) Question: 5. And, does the list of devices in wbsio(4) deliberately exclude the rest of the lm(4) devices? Or could some/all of the remaining chips from lm(4) be added to wbsio(4)? - | Paul Goyette | PGP DSS Key fingerprint: | E-mail addresses: | | Customer Service | FA29 0E3B 35AF E8AE 6651 | paul at whooppee.com | | Network Engineer | 0786 F758 55DE 53BA 7731 | pgoyette at juniper.net | | Kernel Developer | | pgoyette at netbsd.org | -
Re: btuart and SOCKET Bluetooth CF
On Tue, 16 Feb 2010, KIYOHARA Takashi wrote:
/* sc_state */
-#define BTUART_RECV_PKT_TYPE 0 /* packet type */
+enum state {
+ BTUART_RECV_PKT_TYPE, /* packet type */
please don't make this unrelated change. I don't like it because sc_state
should really become "enum state" rather than int but then you need to add
forward declarations..
(btw, no comma is permitted on final enum values)
--- btattach.c 6 Dec 2009 12:55:46 - 1.5
+++ btattach.c 16 Feb 2010 03:28:53 -
@@ -54,13 +54,6 @@
const struct devtype types[] = {
{
- .name = "bcsp",
- .line = "bcsp",
- .descr = "Generic BlueCore Serial Protocol",
- .cflag = CRTSCTS,
- .speed = B57600,
-},
-{
.name = "bcm2035",
.line = "btuart",
.descr = "Broadcom BCM2035",
@@ -68,6 +61,13 @@
.speed = B115200,
},
{
+ .name = "bcsp",
+ .line = "bcsp",
+ .descr = "Generic BlueCore Serial Protocol",
+ .cflag = CRTSCTS,
+ .speed = B57600,
+},
+{
.name = "bgb2xx",
.line = "btuart",
.descr = "Philips BGB2xx module",
@@ -83,7 +83,7 @@
{
.name = "csr",
.line = "btuart",
- .descr = "CSR Casira serial adaptor",
+ .descr = "Cambridge Silicon Radio based modules (not BCSP)",
.init = &init_csr,
.cflag = CRTSCTS,
.speed = B57600,
This is unrelated, and I did it just now..
+static int
+btuart_dtl_probe(dev_t dev)
+{
+#if NCOM > 0
+ struct pcmcia_softc *pcmcia;
+ struct pcmcia_card *card;
+ device_t com;
+ extern struct cdevsw com_cdevsw;
+ struct {
+ uint32_t vendor;
+ uint32_t product;
+ } dtltbl[] = {
+ { PCMCIA_VENDOR_SOCKET, 0x009f },
+ };
+ int i;
+
+ if (major(dev) != cdevsw_lookup_major(&com_cdevsw))
+ return 0;
+
+ com = device_find_by_driver_unit("com", minor(dev));
+ if (device_is_a(device_parent(com), "pcmcia")) {
+ pcmcia = device_private(device_parent(com));
+ card = &pcmcia->card;
+
+ for (i = 0; i < __arraycount(dtltbl); i++)
+ if (card->manufacturer == dtltbl[i].vendor &&
+ card->product == dtltbl[i].product)
+ return 1;
+ }
+#endif
+ return 0;
+}
I really dislike this function, it depends on internals of com driver [btw
COMUNIT(x) would be (minor(x) & COMUNIT_MASK)] and pcmcia and using this
method prevents a soft emulator and/or needs kernel modifications to
handle any new device.
I still think using .l_name = "btdtl" is the best method to select DTL
protocol..
iain
