linux-bluetooth.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Marcel Holtmann <marcel@holtmann.org>
To: "Savoy, Pavan" <pavan_savoy@ti.com>
Cc: "linux-bluetooth@vger.kernel.org"
	<linux-bluetooth@vger.kernel.org>,
	"johan.hedberg@gmail.com" <johan.hedberg@gmail.com>,
	"greg@kroah.com" <greg@kroah.com>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: RE: [PATCH 1/2] drivers:bluetooth: TI_ST bluetooth driver
Date: Thu, 07 Oct 2010 17:37:23 +0200	[thread overview]
Message-ID: <1286465843.6145.157.camel@aeonflux> (raw)
In-Reply-To: <19F8576C6E063C45BE387C64729E739404AA21D1B4@dbde02.ent.ti.com>

Hi Pavan,

> > > > Registering the Bluetooth HCI driver in module_init/module_exit is not
> > > > acceptable. Turn your shared transport into a proper bus.
> > >
> > > Yes, you did comment on it before, I remember, I did prototype the driver as
> > > a bus driver, However I didn't find any advantages by converting it to a bus
> > > driver.
> > > As in, currently the shared transport driver is a line discipline driver
> > because
> > > it is the only way it can communicate over TTY without being tightly coupled
> > with the UART driver.
> > >
> > > > We want to be able to have generic kernels where this module is enabled,
> > > > but no Shared Transport is available.
> > >
> > > Oh if this is the reason I cannot have hci_register/_unregister in
> > module_init/_exit, Can I do this module "depends" on TI_ST, Then it would not
> > > even be visible to build if TI_ST is not selected.
> > 
> > this is not helping either. Then TI_ST can not be selected and so you
> > still end up with some weird platform specific kernels. We don't want
> > that. We want generic kernels that can detect the hardware they are
> > running on.
> > 
> > As I said, I will not accept this driver if it registers HCI device in
> > module_init. No other driver is doing this and it is in general a really
> > really really bad idea.
> > 
> 
> Ok, now I am beginning to get what you say, Let me check, may be what
> I can do is, have something like a st_prepare() function called in the
> module_init, and a _probe function of the bluetooth driver will be called,
> _ONLY_ if the _probe of my platform driver has been called..
> Do you think this would be a good idea?
> 
> Note: the TI_ST driver is also a platform device driver, so that TI_ST's
> Probe is not called, if a arch/xx/board-xx doesn't add it.

that that should be your bus right there.

Let me repeat this. If you register the HCI device in module_init then
it will be registered on all platform this module is selected. Even if
the kernel runs on x86. And that is not acceptable. Registering devices
in module_init is a bad idea no matter what. That is why all other
drivers just register a driver here and not a device.

Regards

Marcel



  reply	other threads:[~2010-10-07 15:37 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-10-06 22:34 [PATCH 0/2] Bluetooth driver for TI WiLink 7 pavan-savoy
2010-10-06 22:34 ` [PATCH 1/2] drivers:bluetooth: TI_ST bluetooth driver pavan-savoy
2010-10-06 22:34   ` [PATCH 2/2] drivers:bluetooth: Kconfig & Makefile for TI BT pavan-savoy
2010-10-07 10:05   ` [PATCH 1/2] drivers:bluetooth: TI_ST bluetooth driver Marcel Holtmann
2010-10-07 14:34     ` Greg KH
2010-10-07 15:21       ` Marcel Holtmann
2010-10-07 15:35         ` Greg KH
2010-10-07 17:57         ` Gustavo F. Padovan
2010-10-07 21:24           ` Savoy, Pavan
2010-10-07 17:51       ` Gustavo F. Padovan
2010-10-07 21:30         ` Greg KH
2010-10-07 19:09           ` Gustavo F. Padovan
2010-10-07 22:17             ` Greg KH
2010-10-07 19:44               ` Gustavo F. Padovan
2010-10-07 14:59     ` Savoy, Pavan
2010-10-07 15:17       ` Marcel Holtmann
2010-10-07 15:23         ` Savoy, Pavan
2010-10-07 15:37           ` Marcel Holtmann [this message]
2010-10-07 15:47             ` Savoy, Pavan
2010-10-07 15:49               ` Marcel Holtmann
2010-10-07 15:52                 ` Savoy, Pavan
  -- strict thread matches above, loose matches on Subject: below --
2010-10-07 18:47 [PATCH 0/2] v2: Bluetooth driver for TI_ST pavan_savoy
2010-10-07 18:47 ` [PATCH 1/2] drivers:bluetooth: TI_ST bluetooth driver pavan_savoy
2010-10-07 18:45   ` Gustavo F. Padovan
2010-10-07 21:53     ` Savoy, Pavan
2010-10-08  8:37     ` Marcel Holtmann
2010-10-08 14:54       ` Savoy, Pavan

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=1286465843.6145.157.camel@aeonflux \
    --to=marcel@holtmann.org \
    --cc=greg@kroah.com \
    --cc=johan.hedberg@gmail.com \
    --cc=linux-bluetooth@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=pavan_savoy@ti.com \
    /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).