Johannes, So it really is a GPIO_RFKILL driver. What's bluetooth specific in it? <Anantha> Main aim of the driver is to provide a single point of entry for all requirements to switch on BT Chip. Not only GPIOs are Handles in this driver, Other resources like "reference clock" requirements are also handled. If required need to add any regulator operations required by BT chip. Please let me know if it still make sense to only have GPIO operations in this driver? GPIO should show up in the name and description. BT should not. The driver should be moved to net/rfkill/ and be generic for GPIO. <Anantha> I can move to net/rfkill/. I don't think all of this is really the best way to do things. This hardcodes "bt_clk" for example. But that's useless for a generic driver. <Anantha> This requires creation of a new header file that is shared between driver and platform. I was trying to eliminate the requirement of a new header file by creating generic platform resources. Will add new header file. -Anantha -----Original Message----- From: Johannes Berg [mailto:johannes@xxxxxxxxxxxxxxxx] Sent: Tuesday, April 12, 2011 5:12 PM To: Anantha Idapalapati Cc: olofj@xxxxxxxxxxxx; linville@xxxxxxxxxxxxx; Uday Raval; linux-wireless@xxxxxxxxxxxxxxx; Andy Ritger; Rakesh Kumar Subject: Re: [PATCH] CHROMIUM: config: bluetooth: rfkill driver On Tue, 2011-04-12 at 16:55 +0530, aidapalapati@xxxxxxxxxx wrote: > From: Anantha Idapalapati <aidapalapati@xxxxxxxxxx> > > Initial version of new "rfkill" driver to control BT radio. > A new kernel config variable CONFIG_BT_RFKILL is defined and > need to be used to include this driver in the kernel. > > Three Platform resources are expected by the driver. > - Shutdown GPIO > - Reset GPIO and > - Reference Clock. > Any/All of the resources can be defined by a platform. So it really is a GPIO_RFKILL driver. What's bluetooth specific in it? > BUG=none > TEST= tested on board using BCM4329 (ventana) > > Change-Id: I38e6ad3a772180b7cab5cf2d59b459b21051817e > --- a/drivers/bluetooth/Kconfig > +++ b/drivers/bluetooth/Kconfig > @@ -219,4 +219,12 @@ config BT_ATH3K > Say Y here to compile support for "Atheros firmware download driver" > into the kernel or say M to compile it as module (ath3k). > > +config BT_RFKILL > + bool "Bluetooth RFKILL driver" > + depends on BT && RFKILL > + help > + If you say yes here you get support of a generic bluetooth RFKILL > + driver for BT chipset. Platform needs to define the resources > + required. GPIO should show up in the name and description. BT should not. The driver should be moved to net/rfkill/ and be generic for GPIO. > +static int bt_rfkill_probe(struct platform_device *pdev) > +{ > + struct rfkill *bt_rfkill_dev; > + struct resource *res; > + int ret; > + bool enable = false; /* off */ > + bool default_sw_block_state; > + > + bt_rfkill = kzalloc(sizeof(*bt_rfkill), GFP_KERNEL); > + if (!bt_rfkill) > + return -ENOMEM; > + > + bt_rfkill->bt_clk = clk_get(&pdev->dev, "bt_clk"); I don't think all of this is really the best way to do things. This hardcodes "bt_clk" for example. But that's useless for a generic driver. Please look at http://article.gmane.org/gmane.linux.kernel/1124137 That driver requires that some specific code registers a platform device with the right data, but you could do that for this as well and get rid of all the hardcoded strings and the hard-coded assumption that it's for bluetooth. I think that approach is MUCH better, since then if somebody has a Wifi or GPS device with GPIO control they can reuse this code. johannes ----------------------------------------------------------------------------------- This email message is for the sole use of the intended recipient(s) and may contain confidential information. Any unauthorized review, use, disclosure or distribution is prohibited. If you are not the intended recipient, please contact the sender by reply email and destroy all copies of the original message. ----------------------------------------------------------------------------------- ÿô.nÇ·®+%˱é¥wÿº{.nÇ·¥{±ÿ«zW¬³ø¡Ü}©²ÆzÚj:+v¨þø®w¥þàÞ¨è&¢)ß«a¶Úÿûz¹ÞúÝjÿwèf