[PATCH][RFC][RFT] ssb: pick PCMCIA host code support from b43 driver

On 23 September 2015 at 17:58, Michael B?sch <[email protected]> wrote:
> On Wed, 23 Sep 2015 12:02:48 +0200
> Rafa? Mi?ecki <[email protected]> wrote:
>
>> On 21 September 2015 at 18:20, Michael B?sch <[email protected]> wrote:
>> > On Mon, 21 Sep 2015 11:04:19 +0200
>> > Rafa? Mi?ecki <[email protected]> wrote:
>> >> @@ -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?
>>
>> I just copied the logic from few lines above where we handle PCI init.
>> I guess the point was to support other host devices even is PCI host
>> registration fails.
>
>
> Ah I misread it. This is at modinit time. That might make sense then.
>
>
>> >> +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.
>>
>> This is something I don't understand. This PCI bridge was also always
>> confusing me.
>> Why do we want a separated file for that? What's wrong with having 1
>> file for host (PCI/PCMCIA) driver (probe and remove functions) *and*
>> ssb initialization?
>
>
> Because that's not ssb code. These are device IDs for b43 devices.
> We just keep it in ssb to make module handling easier.
> Ssb also runs non-b43 devices.
> Think of it like PCI IDs that belong into the driver and not the PCI
> subsystem.

Sorry, it's still unclear for me.

If all PCI-bus-host-specific code is always the same, what's the point
of having separated files for devices with different functionality?
Now I can see we have:
1) b43_pci_ssb_bridge_init
2) b44_pci_init
which do the same thing. Why we can't simply put that code in ssb
itself, e.g. pcihost_wrapper.c? It looks like adding extra unneeded
logic into drivers like b44.

-- 
Rafa?