All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Michael Büsch" <m@bues.ch>
To: "Rafał Miłecki" <zajec5@gmail.com>
Cc: linux-wireless@vger.kernel.org,
	Larry Finger <larry.finger@lwfinger.net>,
	Hauke Mehrtens <hauke@hauke-m.de>,
	b43-dev@lists.infradead.org
Subject: [PATCH][RFC][RFT] ssb: pick PCMCIA host code support from b43 driver
Date: Mon, 21 Sep 2015 18:20:00 +0200	[thread overview]
Message-ID: <20150921182000.6d89445d@wiggum> (raw)
In-Reply-To: <1442826259-6270-1-git-send-email-zajec5@gmail.com>

On Mon, 21 Sep 2015 11:04:19 +0200
Rafa? Mi?ecki <zajec5@gmail.com> wrote:

> ssb bus can be found on various "host" devices like PCI/PCMCIA/SDIO.
> Every ssb bus contains cores AKA devices.
> The main idea is to have ssb driver scan/initialize bus and register
> ready-to-use cores. This way ssb drivers can operate on a single core
> mostly ignoring underlaying details.
> 
> For some reason PCMCIA support was split between ssb and b43. We got
> PCMCIA host device probing in b43, then bus scanning in ssb and then
> wireless core probing back in b43. The truth is it's very unlikely we
> will ever see PCMCIA ssb device with no 802.11 core but I still don't
> see any advantage of the current architecture.

The idea basically was that b43 is the only user of that code. So the
code was put there.

> With proposed change we get the same functionality with a simpler
> architecture, less Kconfig symbols, one killed EXPORT and hopefully
> cleaner b43. Since b43 supports both: ssb & bcma I prefer to keep ssb
> specific code in ssb driver.

I agree that this makes the architecture a bit cleaner. So this
basically looks good. I currently can't test it, because I don't have
that device here right now. In two weeks or so I'll probably be able to
test it, though.


> @@ -1464,6 +1463,12 @@ static int __init ssb_modinit(void)
>  		/* don't fail SSB init because of this */
>  		err = 0;
>  	}
> +	err = ssb_host_pcmcia_init();
> +	if (err) {
> +		ssb_err("PCMCIA host initialization failed\n");
> +		/* don't fail SSB init because of this */

Why not? What's the point of not failing here?

> +		err = 0;
> +	}


> +static const struct pcmcia_device_id ssb_host_pcmcia_tbl[] = {
> +	PCMCIA_DEVICE_MANF_CARD(0x2D0, 0x448),
> +	PCMCIA_DEVICE_MANF_CARD(0x2D0, 0x476),
> +	PCMCIA_DEVICE_NULL,
> +};

This doesn't belong into ssb'c pcmcia.c, IMO.
It should be in a new file called b43_pcmcia_bridge.c, just like we have
b43_pci_bridge.c.
The bridge code technically (also for pci) doesn't belong into ssb. But
it makes kconfig simpler.

-- 
Michael
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 819 bytes
Desc: OpenPGP digital signature
URL: <http://lists.infradead.org/pipermail/b43-dev/attachments/20150921/1cb46e15/attachment.sig>

WARNING: multiple messages have this Message-ID (diff)
From: "Michael Büsch" <m@bues.ch>
To: "Rafał Miłecki" <zajec5@gmail.com>
Cc: linux-wireless@vger.kernel.org,
	Larry Finger <larry.finger@lwfinger.net>,
	Hauke Mehrtens <hauke@hauke-m.de>,
	b43-dev@lists.infradead.org
Subject: Re: [PATCH][RFC][RFT] ssb: pick PCMCIA host code support from b43 driver
Date: Mon, 21 Sep 2015 18:20:00 +0200	[thread overview]
Message-ID: <20150921182000.6d89445d@wiggum> (raw)
In-Reply-To: <1442826259-6270-1-git-send-email-zajec5@gmail.com>

[-- Attachment #1: Type: text/plain, Size: 2109 bytes --]

On Mon, 21 Sep 2015 11:04:19 +0200
Rafał Miłecki <zajec5@gmail.com> wrote:

> ssb bus can be found on various "host" devices like PCI/PCMCIA/SDIO.
> Every ssb bus contains cores AKA devices.
> The main idea is to have ssb driver scan/initialize bus and register
> ready-to-use cores. This way ssb drivers can operate on a single core
> mostly ignoring underlaying details.
> 
> For some reason PCMCIA support was split between ssb and b43. We got
> PCMCIA host device probing in b43, then bus scanning in ssb and then
> wireless core probing back in b43. The truth is it's very unlikely we
> will ever see PCMCIA ssb device with no 802.11 core but I still don't
> see any advantage of the current architecture.

The idea basically was that b43 is the only user of that code. So the
code was put there.

> With proposed change we get the same functionality with a simpler
> architecture, less Kconfig symbols, one killed EXPORT and hopefully
> cleaner b43. Since b43 supports both: ssb & bcma I prefer to keep ssb
> specific code in ssb driver.

I agree that this makes the architecture a bit cleaner. So this
basically looks good. I currently can't test it, because I don't have
that device here right now. In two weeks or so I'll probably be able to
test it, though.


> @@ -1464,6 +1463,12 @@ static int __init ssb_modinit(void)
>  		/* don't fail SSB init because of this */
>  		err = 0;
>  	}
> +	err = ssb_host_pcmcia_init();
> +	if (err) {
> +		ssb_err("PCMCIA host initialization failed\n");
> +		/* don't fail SSB init because of this */

Why not? What's the point of not failing here?

> +		err = 0;
> +	}


> +static const struct pcmcia_device_id ssb_host_pcmcia_tbl[] = {
> +	PCMCIA_DEVICE_MANF_CARD(0x2D0, 0x448),
> +	PCMCIA_DEVICE_MANF_CARD(0x2D0, 0x476),
> +	PCMCIA_DEVICE_NULL,
> +};

This doesn't belong into ssb'c pcmcia.c, IMO.
It should be in a new file called b43_pcmcia_bridge.c, just like we have
b43_pci_bridge.c.
The bridge code technically (also for pci) doesn't belong into ssb. But
it makes kconfig simpler.

-- 
Michael

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

  parent reply	other threads:[~2015-09-21 16:20 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-09-21  9:04 [PATCH][RFC][RFT] ssb: pick PCMCIA host code support from b43 driver Rafał Miłecki
2015-09-21  9:04 ` Rafał Miłecki
2015-09-21 16:14 ` Larry Finger
2015-09-21 16:14   ` Larry Finger
2015-09-21 16:26   ` Michael Büsch
2015-09-21 16:26     ` Michael Büsch
2015-09-21 16:38     ` Larry Finger
2015-09-21 16:38       ` Larry Finger
2015-09-21 16:20 ` Michael Büsch [this message]
2015-09-21 16:20   ` Michael Büsch
2015-09-23 10:02   ` Rafał Miłecki
2015-09-23 10:02     ` Rafał Miłecki
2015-09-23 15:58     ` Michael Büsch
2015-09-23 15:58       ` Michael Büsch
2015-10-14 11:17       ` Rafał Miłecki
2015-10-14 11:17         ` Rafał Miłecki
2015-10-14 14:48         ` Michael Büsch
2015-10-14 14:48           ` Michael Büsch

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=20150921182000.6d89445d@wiggum \
    --to=m@bues.ch \
    --cc=b43-dev@lists.infradead.org \
    --cc=hauke@hauke-m.de \
    --cc=larry.finger@lwfinger.net \
    --cc=linux-wireless@vger.kernel.org \
    --cc=zajec5@gmail.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.