2011/5/8 Arnd Bergmann <arnd@xxxxxxxx>:
> On Sunday 08 May 2011 16:59:55 RafaÅ MiÅecki wrote:
>> 2011/5/8 Arnd Bergmann <arnd@xxxxxxxx>:
>> >> >> 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Å
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
