Re: [PATCH][WAS:bcmai, axi] bcma: add Broadcom specific AMBA bus driver

["Rafał Miłecki" <zajec5@gmail.com>]

2011/5/8 Arnd Bergmann <arnd@arndb.de>:
> On Sunday 08 May 2011 16:59:55 Rafał Miłecki wrote:
>> 2011/5/8 Arnd Bergmann <arnd@arndb.de>:
>> >> >> diff --git a/drivers/bcma/TODO b/drivers/bcma/TODO
>> >> >> new file mode 100644
>> >> >> index 0000000..45eadc9
>> >> >> --- /dev/null
>> >> >> +++ b/drivers/bcma/TODO
>> >> >> @@ -0,0 +1,3 @@
>> >> >> +- Interrupts
>> >> >> +- Defines for PCI core driver
>> >> >> +- Convert bcma_bus->cores into linked list
>> >> >
>> >> > The last item doesn't make sense to me. Since you are using the regular
>> >> > driver model, you can simply iterate over all child devices of any
>> >> > dev.
>> >>
>> >> It's about optimization. Right now bcma_bus->cores is static array, we
>> >> probably never will use all entries.
>> >
>> > Oh, I see. You should probably have neither of them. Instead allocate
>> > the devices dynamically as you find them and do a device_register,
>> > which will add the device into linked list.
>>
>> As I said, and wrote: TODO.
>
> Well, I think getting this part right is essential before the
> patch can get merged.
>
>> > Maybe you didn't understand what I said: This should be
>> >
>> > struct bcma_device {
>> >     struct bcma_bus *bus;
>> >     struct bcma_device_id id;
>> >     struct device dev;
>> >     u8 core_index;
>> >
>> >     u32 addr;
>> >     u32 wrap;
>> >
>> >     void *drvdata;
>> > };
>> >
>> > Here, bcma_device is the device, no need to follow pointers
>> > around. It's how all bus_types work, you should just do the same.
>>
>> We can not use static "struct device", see Greg's comments in:
>> [RFC][PATCH V3] axi: add AXI bus driver
>> (not to mention we would have unused "struct device" in ChipCommon's
>> and PCI's "struct bcma_device").
>
> Please reread what Greg explained, it's actually the same as what
> I said here: Don't make the device static (you already don't),
> don't put the device structure as a member in the bus structure
> (as discussed above). Make the device a member of bcma_device,
> so you get proper reference counting for it, in the way that
> Greg explained.

Thanks for help & explaining! Unfortunately Greg didn't answer if my
changed implementation is fine. I'll fix this!

-- 
Rafał