linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
From: zajec5@gmail.com (Rafał Miłecki)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH][WAS:bcmai, axi] bcma: add Broadcom specific AMBA bus driver
Date: Mon, 9 May 2011 16:33:43 +0200	[thread overview]
Message-ID: <BANLkTikoLnrVvAWC7v7+q1fe4+Qt3HF4ug@mail.gmail.com> (raw)
In-Reply-To: <201105081759.03032.arnd@arndb.de>

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?

  reply	other threads:[~2011-05-09 14:33 UTC|newest]

Thread overview: 45+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-05-05 21:59 [PATCH][WAS:bcmai,axi] bcma: add Broadcom specific AMBA bus driver Rafał Miłecki
2011-05-05 22:59 ` [PATCH][WAS:bcmai, axi] " Julian Calaby
2011-05-05 23:01   ` Rafał Miłecki
2011-05-06 14:05 ` Arnd Bergmann
2011-05-06 14:50   ` Rafał Miłecki
2011-05-07 13:34     ` Rafał Miłecki
2011-05-07 13:55       ` [PATCH][WAS:bcmai,axi] " Michael Büsch
2011-05-07 16:29         ` Arend van Spriel
2011-05-07 16:49           ` [PATCH][WAS:bcmai, axi] " Rafał Miłecki
2011-05-07 17:04             ` [PATCH][WAS:bcmai,axi] " Arend van Spriel
2011-05-07 17:20               ` [PATCH][WAS:bcmai, axi] " Rafał Miłecki
2011-05-07 17:51       ` [PATCH][WAS:bcmai,axi] " George Kashperko
2011-05-07 18:05         ` [PATCH][WAS:bcmai, axi] " Rafał Miłecki
2011-05-07 18:26           ` [PATCH][WAS:bcmai,axi] " George Kashperko
2011-05-07 18:48             ` [PATCH][WAS:bcmai, axi] " Rafał Miłecki
2011-05-07 19:02               ` [PATCH][WAS:bcmai,axi] " George Kashperko
2011-05-07 19:21                 ` [PATCH][WAS:bcmai, axi] " Rafał Miłecki
2011-05-07 19:35                   ` [PATCH][WAS:bcmai,axi] " George Kashperko
2011-05-08  1:44                     ` Michael Büsch
2011-05-08  2:01                       ` George Kashperko
2011-05-07 19:03               ` George Kashperko
2011-05-08  8:43               ` Arend van Spriel
2011-05-08 10:16               ` Russell King - ARM Linux
2011-05-08 10:37                 ` [PATCH][WAS:bcmai, axi] " Rafał Miłecki
2011-05-08 10:50                   ` [PATCH][WAS:bcmai,axi] " Russell King - ARM Linux
2011-05-08 15:06       ` [PATCH][WAS:bcmai, axi] " Arnd Bergmann
2011-05-08 15:25         ` Rafał Miłecki
2011-05-08 15:54           ` Arnd Bergmann
2011-05-08 14:47     ` Arnd Bergmann
2011-05-08 14:59       ` Rafał Miłecki
2011-05-08 15:59         ` Arnd Bergmann
2011-05-09 14:33           ` Rafał Miłecki [this message]
2011-05-09 15:37             ` Greg KH
2011-05-09 15:48               ` Rafał Miłecki
2011-05-07 16:13 ` Hauke Mehrtens
2011-05-07 16:23   ` Rafał Miłecki
2011-05-07 16:32     ` Hauke Mehrtens
2011-05-07 16:51       ` Rafał Miłecki
2011-05-07 17:24         ` Hauke Mehrtens
2011-05-07 17:35           ` Rafał Miłecki
2011-05-07 17:45           ` [PATCH][WAS:bcmai,axi] " George Kashperko
2011-05-07 22:42           ` Henry Ptasinski
2011-05-07 23:17             ` [PATCH][WAS:bcmai, axi] " Hauke Mehrtens
2011-05-08 12:48 ` Hauke Mehrtens
2011-05-08 12:55   ` Rafał Miłecki

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=BANLkTikoLnrVvAWC7v7+q1fe4+Qt3HF4ug@mail.gmail.com \
    --to=zajec5@gmail.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    /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).