2011-03-19 12:14:06

by Arend van Spriel

[permalink] [raw]
Subject: new utility kernel module for detecting cores in newer chipsets

+ mailing lists

Hi Greg,

There has been several discussion in the community (mainly by b43 and ssb
developers, I believe) to add support for chip core interconnect used in
newer Broadcom chips. Our brcm80211 drivers already have this support and
we have isolated that functionality in a separate kernel module called
brcmaxi to provide it to other drivers. Currently, I have it located under
drivers/staging/brcm80211 as I am not sure whether or not its purpose
justifies a separate folder (under staging or not).

What are your thoughts about this? I would like to know before submitting
a patch for this.

Gr. AvS


--
"The most merciful thing in the world, I think, is the inability of the
human
mind to correlate all its contents." - "The Call of Cthulhu"



2011-03-21 15:29:47

by Greg KH

[permalink] [raw]
Subject: Re: new utility kernel module for detecting cores in newer chipsets

On Mon, Mar 21, 2011 at 03:52:56PM +0100, Rafał Miłecki wrote:
> 2011/3/21 Greg KH <[email protected]>:
> > I have no objections to what you want at all, why do you think I do?
> > I'm just handling getting the stuff in drivers/staging/ out of there,
> > that's it.  I trust the rest of you all to handle the design decisions
> > properly.
> >
> > And as there is no code here yet to complain about, I fail to see any
> > problem yet :)
>
> It was not directly to you, I believe you already many things to worry
> about, and are not really interested in getting into another driver
> layout :)

Then why was I the only one on the "To:" line? Please be more careful
in the future.

> I expected Michael and Arend to talk with us (mostly me and George)
> about current situation.

For some reason, there was a seprate thread without the mailing list on
it about this topic where Michael did discuss this. Maybe he will
summarize that here for everyone else now.

thanks,

greg k-h

2011-03-20 13:08:27

by George Kashperko

[permalink] [raw]
Subject: Re: new utility kernel module for detecting cores in newer chipsets

> 2011/3/19 Arend van Spriel <[email protected]>:
> > + mailing lists
> >
> > Hi Greg,
> >
> > There has been several discussion in the community (mainly by b43 and ssb
> > developers, I believe) to add support for chip core interconnect used in
> > newer Broadcom chips. Our brcm80211 drivers already have this support and
> > we have isolated that functionality in a separate kernel module called
> > brcmaxi to provide it to other drivers. Currently, I have it located under
> > drivers/staging/brcm80211 as I am not sure whether or not its purpose
> > justifies a separate folder (under staging or not).
> >
> > What are your thoughts about this? I would like to know before submitting
> > a patch for this.
>
> Well, I've this AI support rewritten and isolated here too. It's not
> about having the code but designing it well.
>
> George Kashperko shared idea of really nice design layout for buses.
> He pointed problems with ssb driver, explained how everything works.
> With his layout it should be easy to support SSB and AI with very
> minimal, if any, additional hacks. Nobody pointed any problems with
> his layout.
Its not just a raw idea. Already implemented it in code. ATM I have bus
glue with three working drivers for chipcommons with pmu r0, r1 and r5,
drivers for mips33k, mips74k and pcie rev.13+ buscommons, embedded and
pcie hosts drivers. Now working on sprom configuration driver.
Still planning for clean glue for PCMCIA/SDIO hosts, some unified
approach for interrupts management and multifunctional pci(e) devices.

And the only one actual problem I faced was not the algorythm for AXI
SROM parsing (which is obvious as soon you figure out the SROM layout
and actually is much simpler and cleaner than that in
brcm80211/utils/aiutils.c)

The problem is technical background information absence for both SB and
AXI. The only truly open doc with SB information is BCM44XX programmers
guide. And for AXI there is nothing at all except staging80211 and
number of GPL'ed firmware sources for embedded routers. Half the time I
spent designing the model was documentation writing which I hate the
most.

Well, I understand its a business model Broadcom tends to run and we
can't do anything with that, but honestly even here I don't really
understand the purpose of "UNPUBLISHED PROPRIETARY" tags in some sources
of GPL'ed firmwares. E. g. etcgmac.c which is the only closed part left
to finally get bcm4716s' supported with openwrt.

> I don't want to have support for AI in 10 places, even if this is
> about staging area.
>
Agree here completely. The only arguable point here I think could be
that AXI and SB should be different drivers but honestly they are too
much similar softwire-wise to even be placed in separate directories if
only bus managing code model is designed well.

Have nice day,
George



2011-03-20 09:55:05

by Rafał Miłecki

[permalink] [raw]
Subject: Re: new utility kernel module for detecting cores in newer chipsets

2011/3/19 Arend van Spriel <[email protected]>:
> + mailing lists
>
> Hi Greg,
>
> There has been several discussion in the community (mainly by b43 and ssb
> developers, I believe) to add support for chip core interconnect used in
> newer Broadcom chips.  Our brcm80211 drivers already have this support and
> we have isolated that functionality in a separate kernel module called
> brcmaxi to provide it to other drivers. Currently, I have it located under
> drivers/staging/brcm80211 as I am not sure whether or not its purpose
> justifies a separate folder (under staging or not).
>
> What are your thoughts about this? I would like to know before submitting
> a patch for this.

Well, I've this AI support rewritten and isolated here too. It's not
about having the code but designing it well.

George Kashperko shared idea of really nice design layout for buses.
He pointed problems with ssb driver, explained how everything works.
With his layout it should be easy to support SSB and AI with very
minimal, if any, additional hacks. Nobody pointed any problems with
his layout.

I don't want to have support for AI in 10 places, even if this is
about staging area.

--
Rafał

2011-03-21 15:15:12

by John W. Linville

[permalink] [raw]
Subject: Re: new utility kernel module for detecting cores in newer chipsets

On Mon, Mar 21, 2011 at 04:05:27PM +0100, Arend van Spriel wrote:

> To summarize this, my main issue (and Michael's, I think) is with
> the dependency being imposed between ai and ssb. Having two
> completely independent modules really makes more sense.

FWIW, this approach makes the most sense to me as well.

--
John W. Linville Someday the world will need a hero, and you
[email protected] might be all we have. Be ready.

2011-03-21 16:28:15

by Michael Büsch

[permalink] [raw]
Subject: Re: new utility kernel module for detecting cores in newer chipsets

On Mon, 2011-03-21 at 17:24 +0100, Michael Büsch wrote:
> On Mon, 2011-03-21 at 16:05 +0100, Arend van Spriel wrote:
> > To summarize this, my main issue (and Michael's, I think) is with the
> > dependency being imposed between ai and ssb.
>
> That's part of my main issue, right.
>
> > Having two completely independent modules really makes more sense.
>
> I do think that any attempt to merge legacy-ssb with ai subsystems
> or even share significant amounts of code between these subsystems
> will result in a maintenance nightmare.
>
> There are times where a fork or a rewrite is the best thing to do.
> And I think that is the case here.
>
> SSB hardware is dead. Let the software die, too.
> You just need to realize that having ai code completely separate from
> ssb code makes life easier for you guys.
> Win: You don't need to take care about backwards compatibility.
> Win: You don't break our legacy devices.
>
> Just look at the patches you guys already sent. Look at that TMSLOW
> abstraction layer, for instance. That's just a pain in the arms.
> Look at that:
> http://permalink.gmane.org/gmane.linux.kernel.wireless.general/66590
> You're carrying old legacy cruft into the shiny new subsystem
> (PCMCIA support, for instance) just to avoid duplicating a trivial
> 100 line function.
>
> Please keep in mind that any attempt to merge SSB with AI code means
> that _you_ guys will have to maintain backwards compatibility
> with devices designed 10 years ago.
>
> I really don't understand why you are so resistant against a fork
> or a rewrite, because it makes _your_ life easier (and mine, too).
>
> PS: I do _not_ know whether the brcmaix code is reasonable or even
> useful
> as base to build up the new broadcom SOC bus subsystem, so I'm not
> going to pass and judgement on that code.
>

I just realized that Arend was in the To: field.
The text was mainly addressed to Rafal and George. Probably Greg as
well.

--
Greetings Michael.


2011-03-21 15:33:28

by Rafał Miłecki

[permalink] [raw]
Subject: Re: new utility kernel module for detecting cores in newer chipsets

2011/3/21 Greg KH <[email protected]>:
> On Mon, Mar 21, 2011 at 03:52:56PM +0100, Rafał Miłecki wrote:
>> 2011/3/21 Greg KH <[email protected]>:
>> > I have no objections to what you want at all, why do you think I do?
>> > I'm just handling getting the stuff in drivers/staging/ out of there,
>> > that's it.  I trust the rest of you all to handle the design decisions
>> > properly.
>> >
>> > And as there is no code here yet to complain about, I fail to see any
>> > problem yet :)
>>
>> It was not directly to you, I believe you already many things to worry
>> about, and are not really interested in getting into another driver
>> layout :)
>
> Then why was I the only one on the "To:" line?  Please be more careful
> in the future.

Just because I hit "Reply" in your e-mail's view. All other were put in CC.


>> I expected Michael and Arend to talk with us (mostly me and George)
>> about current situation.
>
> For some reason, there was a seprate thread without the mailing list on
> it about this topic where Michael did discuss this.  Maybe he will
> summarize that here for everyone else now.

It was on linux-wireless which mostly matches AI usage.
http://comments.gmane.org/gmane.linux.kernel.wireless.general/66590

--
Rafał

2011-03-20 17:04:38

by Arend van Spriel

[permalink] [raw]
Subject: Re: new utility kernel module for detecting cores in newer chipsets

On Sun, 20 Mar 2011 14:07:18 +0100, George Kashperko <[email protected]>
wrote:

>> I don't want to have support for AI in 10 places, even if this is
>> about staging area.
>>
> Agree here completely. The only arguable point here I think could be
> that AXI and SB should be different drivers but honestly they are too
> much similar softwire-wise to even be placed in separate directories if
> only bus managing code model is designed well.

Agree and a valid point at that as one would typically need either ssb or
axi support. If with a little refactoring the ssb tree can provide two
separate drivers that is fine by me if the axi flavour is not burdened by
shortcomings of the the ssb driver, which you are said to have pointed out
(I missed most of the discussion about this, sorry). In brcm80211 we have
two drivers and one typically needs the ssb support and the other axi
support. We abstracted the core access (not judging how well we did that)
with an additional layer but there is actually no need when having
separate drivers requiring either one or the other interconnect support.

Depending on the driver it may depend on one of them or both, but with two
separate modules this choice is up to the driver which makes most sense to
me.

Gr. AvS
--
"The most merciful thing in the world, I think, is the inability of the
human
mind to correlate all its contents." - "The Call of Cthulhu"


2011-03-21 14:45:06

by Greg KH

[permalink] [raw]
Subject: Re: new utility kernel module for detecting cores in newer chipsets

On Mon, Mar 21, 2011 at 03:27:49PM +0100, Rafał Miłecki wrote:
> 2011/3/21 Greg KH <[email protected]>:
> > On Mon, Mar 21, 2011 at 11:12:55AM +0100, Rafał Miłecki wrote:
> >> Sorry, but you seem to be deaf to my and George's comments.
> >
> > Me?
> >
> >> What's wrong with that?
> >
> > What's wrong with what?  You provided no context in this message, so I
> > have absolutely no idea what you are referring to.
> >
> > totally confused,
>
> Did you receive two following e-mails (in this thread):
> 1) From: Rafał Miłecki <[email protected]>
> Date: 20 03 2011 10:55
> 2) From: George Kashperko <[email protected]>
> Date: 20 03 2011 14:07
> ?

In digging through my archives, yes I did.

But the proper thing to do is to quote portions in which you refer to
when talking about them. Otherwise the reciever can easily be lost.

Remember, some of us get _thousands_ of emails a day. Remembering
context based on a subject: line is impossible, especially when the
thread is long gone from my inbox and buried in my archive.

> The case is, we discussed ssb/ai driver layout few days ago. George
> shared idea of layout I agree with, nobody shared any objections. If
> everything goes fine, we should have nicely modularized driver/project
> supporting Broadcom's buses.
>
> In this situation I'm not really interested is simple ai driver
> stripped from brcm80211. According to me, it would led to harder
> maintenance and harder implementation of support for such a driver in
> b43.

Ok, and I trust that you and Michael will work things out.

I have no objections to what you want at all, why do you think I do?
I'm just handling getting the stuff in drivers/staging/ out of there,
that's it. I trust the rest of you all to handle the design decisions
properly.

And as there is no code here yet to complain about, I fail to see any
problem yet :)

thanks,

greg k-h

2011-03-21 16:26:50

by George Kashperko

[permalink] [raw]
Subject: Re: new utility kernel module for detecting cores in newer chipsets

> 2011/3/21 Arend van Spriel <[email protected]>:
> > On Mon, 21 Mar 2011 15:27:49 +0100, Rafał Miłecki <[email protected]> wrote:
> >
> >> The case is, we discussed ssb/ai driver layout few days ago. George
> >> shared idea of layout I agree with, nobody shared any objections. If
> >> everything goes fine, we should have nicely modularized driver/project
> >> supporting Broadcom's buses.
> >
> > Hi Rafał,
> >
> > Does this give us one module supporting both buses or does it provide two
> > kernel modules (my preference)? I did ask you and George this question
> > earlier but I seem to have missed the response from you or George.
>
> I'm still waiting for George to respond and share his code.
>
>
> >> In this situation I'm not really interested is simple ai driver
> >> stripped from brcm80211. According to me, it would led to harder
> >> maintenance and harder implementation of support for such a driver in
> >> b43.
> >
> > I can imagine from b43 perspective the only good implementation would be to
> > stick with the current b43<->ssb interface. So what will you do when another
> > type of SoC interconnect is introduced. Forcing that in the same API as
> > well? If you and George propose a new carefully considered API covering the
> > functional capabilities of the current (and possibly future) interconnect
> > buses I am all for that.
> >
> > To summarize this, my main issue (and Michael's, I think) is with the
> > dependency being imposed between ai and ssb. Having two completely
> > independent modules really makes more sense.
>
> This really depends on new interconnect. If it will be totally
> different, I'll vote for totally different driver. In case of SSB and
> AI, driver layout is similar, it seems George managed to write sth
> nice.
>
> George?
Hi there. Much surprised with such a resonance on the topic. Sorry for
being so late with participating with discussion - work background
requires me sometimes to move around alot - this week is the time for
trips.
If you extremely hurry with that I will make my testings sources
available lately this evening as soon as get to my workpad. But just to
warn you its not that I planned for final "release" just glue I used to
work edge things out. As for final code I plan to finish with it next
weekends - yet again I will move al lot this week and will have much
time to think on the subject but not actually work on it.

Yet again sorry for making you wait.

Now back to subject. I curious about sertain things I see here which
make the most contradictary ones. That ones regarding why AI and SB
should be separate modules etc.
Would be much appreciated if you hear my mind on that (under hear I mean
not just read). Some time ago here in earlier AI rfcs' threads I already
tried to cover the reason why am I sure they should not just share
single code base but also be coupled together.
AI and SB interconnects are really totally diffeent hardware base. But
they stick to the single architecture model. Very much the same as PCI
and PCIE are (see what I'm trying to do here?). This model is not AI or
SB exclusive and in short looks like following (here I'm sorry for
making you read the things you know already, but I do it just because I
see that ones of you see on subject through Broadcom SDK while others do
the same through linux/ssb):
Bus devices coupled under the entity we tend to call cores. The cores
managed from the system bus side with means of the host. The cores
managed from the backplane side with means of agents. The core devices
exposed to system and managed with same drivers regardless the
backplane. The agents system dont supposted to know about and doing the
same enable/disable/reset but in somewhat different way. The fact is (as
at least I see it) that here we want to keep things apart just because
one set of agents is called SB (or e. g. PCI) while the other is called
AXI (or lets say PCIE).
Yes, linux/ssb model design can't cover this. The linux/bcmb (or hnd as
I finally named it - grrr really dont like these 3 letters -.-) might do
better as it is designed to do that. Btw in way which is completely
different from ssb and Broadcom SDK one.

Here much sorry need to get back to work. Will be back late in evening.

Have nice day,
George



2011-03-21 15:30:16

by Rafał Miłecki

[permalink] [raw]
Subject: Re: new utility kernel module for detecting cores in newer chipsets

2011/3/21 Arend van Spriel <[email protected]>:
> On Mon, 21 Mar 2011 15:27:49 +0100, Rafał Miłecki <[email protected]> wrote:
>
>> The case is, we discussed ssb/ai driver layout few days ago. George
>> shared idea of layout I agree with, nobody shared any objections. If
>> everything goes fine, we should have nicely modularized driver/project
>> supporting Broadcom's buses.
>
> Hi Rafał,
>
> Does this give us one module supporting both buses or does it provide two
> kernel modules (my preference)? I did ask you and George this question
> earlier but I seem to have missed the response from you or George.

I'm still waiting for George to respond and share his code.


>> In this situation I'm not really interested is simple ai driver
>> stripped from brcm80211. According to me, it would led to harder
>> maintenance and harder implementation of support for such a driver in
>> b43.
>
> I can imagine from b43 perspective the only good implementation would be to
> stick with the current b43<->ssb interface. So what will you do when another
> type of SoC interconnect is introduced. Forcing that in the same API as
> well? If you and George propose a new carefully considered API covering the
> functional capabilities of the current (and possibly future) interconnect
> buses I am all for that.
>
> To summarize this, my main issue (and Michael's, I think) is with the
> dependency being imposed between ai and ssb. Having two completely
> independent modules really makes more sense.

This really depends on new interconnect. If it will be totally
different, I'll vote for totally different driver. In case of SSB and
AI, driver layout is similar, it seems George managed to write sth
nice.

George?

--
Rafał

2011-03-21 14:14:57

by Greg KH

[permalink] [raw]
Subject: Re: new utility kernel module for detecting cores in newer chipsets

On Mon, Mar 21, 2011 at 11:12:55AM +0100, Rafał Miłecki wrote:
> Sorry, but you seem to be deaf to my and George's comments.

Me?

> What's wrong with that?

What's wrong with what? You provided no context in this message, so I
have absolutely no idea what you are referring to.

totally confused,

greg k-h

2011-03-21 14:52:58

by Rafał Miłecki

[permalink] [raw]
Subject: Re: new utility kernel module for detecting cores in newer chipsets

2011/3/21 Greg KH <[email protected]>:
> In digging through my archives, yes I did.
>
> But the proper thing to do is to quote portions in which you refer to
> when talking about them.  Otherwise the reciever can easily be lost.
>
> Remember, some of us get _thousands_ of emails a day.  Remembering
> context based on a subject: line is impossible, especially when the
> thread is long gone from my inbox and buried in my archive.

Right, sorry for that.


>> The case is, we discussed ssb/ai driver layout few days ago. George
>> shared idea of layout I agree with, nobody shared any objections. If
>> everything goes fine, we should have nicely modularized driver/project
>> supporting Broadcom's buses.
>>
>> In this situation I'm not really interested is simple ai driver
>> stripped from brcm80211. According to me, it would led to harder
>> maintenance and harder implementation of support for such a driver in
>> b43.
>
> Ok, and I trust that you and Michael will work things out.
>
> I have no objections to what you want at all, why do you think I do?
> I'm just handling getting the stuff in drivers/staging/ out of there,
> that's it.  I trust the rest of you all to handle the design decisions
> properly.
>
> And as there is no code here yet to complain about, I fail to see any
> problem yet :)

It was not directly to you, I believe you already many things to worry
about, and are not really interested in getting into another driver
layout :)

I expected Michael and Arend to talk with us (mostly me and George)
about current situation.

--
Rafał

2011-03-21 14:27:51

by Rafał Miłecki

[permalink] [raw]
Subject: Re: new utility kernel module for detecting cores in newer chipsets

2011/3/21 Greg KH <[email protected]>:
> On Mon, Mar 21, 2011 at 11:12:55AM +0100, Rafał Miłecki wrote:
>> Sorry, but you seem to be deaf to my and George's comments.
>
> Me?
>
>> What's wrong with that?
>
> What's wrong with what?  You provided no context in this message, so I
> have absolutely no idea what you are referring to.
>
> totally confused,

Did you receive two following e-mails (in this thread):
1) From: Rafał Miłecki <[email protected]>
Date: 20 03 2011 10:55
2) From: George Kashperko <[email protected]>
Date: 20 03 2011 14:07
?

The case is, we discussed ssb/ai driver layout few days ago. George
shared idea of layout I agree with, nobody shared any objections. If
everything goes fine, we should have nicely modularized driver/project
supporting Broadcom's buses.

In this situation I'm not really interested is simple ai driver
stripped from brcm80211. According to me, it would led to harder
maintenance and harder implementation of support for such a driver in
b43.

--
Rafał

2011-03-21 15:06:15

by Arend van Spriel

[permalink] [raw]
Subject: Re: new utility kernel module for detecting cores in newer chipsets

On Mon, 21 Mar 2011 15:27:49 +0100, Rafał Miłecki <[email protected]> wrote:

> The case is, we discussed ssb/ai driver layout few days ago. George
> shared idea of layout I agree with, nobody shared any objections. If
> everything goes fine, we should have nicely modularized driver/project
> supporting Broadcom's buses.

Hi Rafał,

Does this give us one module supporting both buses or does it provide two
kernel modules (my preference)? I did ask you and George this question
earlier but I seem to have missed the response from you or George.

> In this situation I'm not really interested is simple ai driver
> stripped from brcm80211. According to me, it would led to harder
> maintenance and harder implementation of support for such a driver in
> b43.

I can imagine from b43 perspective the only good implementation would be
to stick with the current b43<->ssb interface. So what will you do when
another type of SoC interconnect is introduced. Forcing that in the same
API as well? If you and George propose a new carefully considered API
covering the functional capabilities of the current (and possibly future)
interconnect buses I am all for that.

To summarize this, my main issue (and Michael's, I think) is with the
dependency being imposed between ai and ssb. Having two completely
independent modules really makes more sense.

Gr. AvS
--
"The most merciful thing in the world, I think, is the inability of the
human
mind to correlate all its contents." - "The Call of Cthulhu"


2011-03-21 10:12:56

by Rafał Miłecki

[permalink] [raw]
Subject: Re: new utility kernel module for detecting cores in newer chipsets

Sorry, but you seem to be deaf to my and George's comments. What's
wrong with that?

--
Rafał

2011-03-21 16:24:48

by Michael Büsch

[permalink] [raw]
Subject: Re: new utility kernel module for detecting cores in newer chipsets

On Mon, 2011-03-21 at 16:05 +0100, Arend van Spriel wrote:
> To summarize this, my main issue (and Michael's, I think) is with the
> dependency being imposed between ai and ssb.

That's part of my main issue, right.

> Having two completely independent modules really makes more sense.

I do think that any attempt to merge legacy-ssb with ai subsystems
or even share significant amounts of code between these subsystems
will result in a maintenance nightmare.

There are times where a fork or a rewrite is the best thing to do.
And I think that is the case here.

SSB hardware is dead. Let the software die, too.
You just need to realize that having ai code completely separate from
ssb code makes life easier for you guys.
Win: You don't need to take care about backwards compatibility.
Win: You don't break our legacy devices.

Just look at the patches you guys already sent. Look at that TMSLOW
abstraction layer, for instance. That's just a pain in the arms.
Look at that:
http://permalink.gmane.org/gmane.linux.kernel.wireless.general/66590
You're carrying old legacy cruft into the shiny new subsystem
(PCMCIA support, for instance) just to avoid duplicating a trivial
100 line function.

Please keep in mind that any attempt to merge SSB with AI code means
that _you_ guys will have to maintain backwards compatibility
with devices designed 10 years ago.

I really don't understand why you are so resistant against a fork
or a rewrite, because it makes _your_ life easier (and mine, too).

PS: I do _not_ know whether the brcmaix code is reasonable or even
useful
as base to build up the new broadcom SOC bus subsystem, so I'm not
going to pass and judgement on that code.

--
Greetings Michael.