linux-sh.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* Re: [PATCH] sh: Enable PMB support on SH4AL-DSP
@ 2011-03-01  6:41 Paul Mundt
  2011-03-01  6:45 ` Magnus Damm
                   ` (7 more replies)
  0 siblings, 8 replies; 9+ messages in thread
From: Paul Mundt @ 2011-03-01  6:41 UTC (permalink / raw)
  To: linux-sh

On Tue, Mar 01, 2011 at 03:45:14PM +0900, Magnus Damm wrote:
> From: Magnus Damm <damm@opensource.se>
> 
> The SH4AL-DSP core included in sh7372 requires use of the PMB,
> update the CONFIG_PMB Kconfig entry to reflect this.
> 
> Signed-off-by: Magnus Damm <damm@opensource.se>

No. SH4AL-DSP does not in general have a PMB. If this CPU subtype is
special, then it needs to be special cased.

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH] sh: Enable PMB support on SH4AL-DSP
  2011-03-01  6:41 [PATCH] sh: Enable PMB support on SH4AL-DSP Paul Mundt
@ 2011-03-01  6:45 ` Magnus Damm
  2011-03-01  8:02 ` Magnus Damm
                   ` (6 subsequent siblings)
  7 siblings, 0 replies; 9+ messages in thread
From: Magnus Damm @ 2011-03-01  6:45 UTC (permalink / raw)
  To: linux-sh

From: Magnus Damm <damm@opensource.se>

The SH4AL-DSP core included in sh7372 requires use of the PMB,
update the CONFIG_PMB Kconfig entry to reflect this.

Signed-off-by: Magnus Damm <damm@opensource.se>
---

 arch/sh/mm/Kconfig |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

--- 0001/arch/sh/mm/Kconfig
+++ work/arch/sh/mm/Kconfig	2011-03-01 15:42:12.000000000 +0900
@@ -83,7 +83,7 @@ config 32BIT
 
 config PMB
 	bool "Support 32-bit physical addressing through PMB"
-	depends on MMU && EXPERIMENTAL && CPU_SH4A && !CPU_SH4AL_DSP
+	depends on MMU && EXPERIMENTAL && CPU_SH4A
 	select 32BIT
 	select UNCACHED_MAPPING
 	help

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] sh: Enable PMB support on SH4AL-DSP
  2011-03-01  6:41 [PATCH] sh: Enable PMB support on SH4AL-DSP Paul Mundt
  2011-03-01  6:45 ` Magnus Damm
@ 2011-03-01  8:02 ` Magnus Damm
  2011-03-01  8:06 ` Paul Mundt
                   ` (5 subsequent siblings)
  7 siblings, 0 replies; 9+ messages in thread
From: Magnus Damm @ 2011-03-01  8:02 UTC (permalink / raw)
  To: linux-sh

On Tue, Mar 1, 2011 at 3:41 PM, Paul Mundt <lethal@linux-sh.org> wrote:
> On Tue, Mar 01, 2011 at 03:45:14PM +0900, Magnus Damm wrote:
>> From: Magnus Damm <damm@opensource.se>
>>
>> The SH4AL-DSP core included in sh7372 requires use of the PMB,
>> update the CONFIG_PMB Kconfig entry to reflect this.
>>
>> Signed-off-by: Magnus Damm <damm@opensource.se>
>
> No. SH4AL-DSP does not in general have a PMB. If this CPU subtype is
> special, then it needs to be special cased.

Well, SH4AL-DSP may not have a PMB in general, but it also may not
"not" have it either.

This is what I can tell after going through data sheet for a bunch of
SH4AL-DSP SoCs:

sh7722 SH4AL-DSP
sh7366 SH4AL-DSP
sh7343 SH4AL-DSP + EXT
sh7367 SH4AL-DSP + EXT + PMB + BOOT
sh7377 SH4AL-DSP + EXT + PMB + BOOT
sh7372 SH4AL-DSP + EXT + PMB + BOOT

EXT = "SH4AL-DSP Extended Functions"
PMB = "32-bit Address Extended Mode"
BOOT = "32-bit Boot Function"

From the table it looks like the SH4AL-DSP core can come with and without PMB.

How would you like to special case it?

/ magnus

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] sh: Enable PMB support on SH4AL-DSP
  2011-03-01  6:41 [PATCH] sh: Enable PMB support on SH4AL-DSP Paul Mundt
  2011-03-01  6:45 ` Magnus Damm
  2011-03-01  8:02 ` Magnus Damm
@ 2011-03-01  8:06 ` Paul Mundt
  2011-03-01  8:40 ` Magnus Damm
                   ` (4 subsequent siblings)
  7 siblings, 0 replies; 9+ messages in thread
From: Paul Mundt @ 2011-03-01  8:06 UTC (permalink / raw)
  To: linux-sh

On Tue, Mar 01, 2011 at 05:02:17PM +0900, Magnus Damm wrote:
> On Tue, Mar 1, 2011 at 3:41 PM, Paul Mundt <lethal@linux-sh.org> wrote:
> > On Tue, Mar 01, 2011 at 03:45:14PM +0900, Magnus Damm wrote:
> >> From: Magnus Damm <damm@opensource.se>
> >>
> >> The SH4AL-DSP core included in sh7372 requires use of the PMB,
> >> update the CONFIG_PMB Kconfig entry to reflect this.
> >>
> >> Signed-off-by: Magnus Damm <damm@opensource.se>
> >
> > No. SH4AL-DSP does not in general have a PMB. If this CPU subtype is
> > special, then it needs to be special cased.
> 
> Well, SH4AL-DSP may not have a PMB in general, but it also may not
> "not" have it either.
> 
> This is what I can tell after going through data sheet for a bunch of
> SH4AL-DSP SoCs:
> 
> sh7722 SH4AL-DSP
> sh7366 SH4AL-DSP
> sh7343 SH4AL-DSP + EXT
> sh7367 SH4AL-DSP + EXT + PMB + BOOT
> sh7377 SH4AL-DSP + EXT + PMB + BOOT
> sh7372 SH4AL-DSP + EXT + PMB + BOOT
> 
> EXT = "SH4AL-DSP Extended Functions"
> PMB = "32-bit Address Extended Mode"
> BOOT = "32-bit Boot Function"
> 
> From the table it looks like the SH4AL-DSP core can come with and without PMB.
> 
That's a pretty special way of interpreting the results.

> How would you like to special case it?
> 
From the above table we see that all SH4AL-DSPs that are standalone cores
have no PMB functionality, and that the only ones that do are in ARM/SH
multi-core configurations. This is what needs to be special cased.
Implying that the potential existence of the PMB has anything at all to
do with SH4AL-DSP outside of the SH/R-Mobile context is disingenous at
best.

If a standalone SH4AL-DSP pops up with PMB functionality in the future
then of course that can be special cased too, but for now implying that
SH4AL-DSP = PMB is absurd.

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] sh: Enable PMB support on SH4AL-DSP
  2011-03-01  6:41 [PATCH] sh: Enable PMB support on SH4AL-DSP Paul Mundt
                   ` (2 preceding siblings ...)
  2011-03-01  8:06 ` Paul Mundt
@ 2011-03-01  8:40 ` Magnus Damm
  2011-03-01  9:02 ` Paul Mundt
                   ` (3 subsequent siblings)
  7 siblings, 0 replies; 9+ messages in thread
From: Magnus Damm @ 2011-03-01  8:40 UTC (permalink / raw)
  To: linux-sh

On Tue, Mar 1, 2011 at 5:06 PM, Paul Mundt <lethal@linux-sh.org> wrote:
> On Tue, Mar 01, 2011 at 05:02:17PM +0900, Magnus Damm wrote:
>> On Tue, Mar 1, 2011 at 3:41 PM, Paul Mundt <lethal@linux-sh.org> wrote:
>> > On Tue, Mar 01, 2011 at 03:45:14PM +0900, Magnus Damm wrote:
>> >> From: Magnus Damm <damm@opensource.se>
>> >>
>> >> The SH4AL-DSP core included in sh7372 requires use of the PMB,
>> >> update the CONFIG_PMB Kconfig entry to reflect this.
>> >>
>> >> Signed-off-by: Magnus Damm <damm@opensource.se>
>> >
>> > No. SH4AL-DSP does not in general have a PMB. If this CPU subtype is
>> > special, then it needs to be special cased.
>>
>> Well, SH4AL-DSP may not have a PMB in general, but it also may not
>> "not" have it either.
>>
>> This is what I can tell after going through data sheet for a bunch of
>> SH4AL-DSP SoCs:
>>
>> sh7722 SH4AL-DSP
>> sh7366 SH4AL-DSP
>> sh7343 SH4AL-DSP + EXT
>> sh7367 SH4AL-DSP + EXT + PMB + BOOT
>> sh7377 SH4AL-DSP + EXT + PMB + BOOT
>> sh7372 SH4AL-DSP + EXT + PMB + BOOT
>>
>> EXT = "SH4AL-DSP Extended Functions"
>> PMB = "32-bit Address Extended Mode"
>> BOOT = "32-bit Boot Function"
>>
>> From the table it looks like the SH4AL-DSP core can come with and without PMB.
>>
> That's a pretty special way of interpreting the results.
>
>> How would you like to special case it?
>>
> From the above table we see that all SH4AL-DSPs that are standalone cores
> have no PMB functionality, and that the only ones that do are in ARM/SH
> multi-core configurations. This is what needs to be special cased.

So the ARM/SH multi-core configurations with SH4A cores do not?

> Implying that the potential existence of the PMB has anything at all to
> do with SH4AL-DSP outside of the SH/R-Mobile context is disingenous at
> best.
> If a standalone SH4AL-DSP pops up with PMB functionality in the future
> then of course that can be special cased too, but for now implying that
> SH4AL-DSP = PMB is absurd.

I'm not implying that. Your logic seems to be inverse somehow. The
patch does not imply "SH4AL-DSP = PMB", instead it simply removes the
assumption "SH4AL-DSP != PMB". You don't want to allow people to
enable PMB on SH4AL-DSP?

Take a look at sh7723. It has an X2 core and selects CPU_SH4A. People
compiling for sh7723 can chose to enable CONFIG_PMB if they want to.
This may not be the way you want it to work though, I'm not sure.

But if the sh7723 case is like that then I can't see what is wrong
with removing the !SH4AL_DSP bits for the Kconfig.

Or you do want something like SYS_SUPPORTS_PMB?

/ magnus

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] sh: Enable PMB support on SH4AL-DSP
  2011-03-01  6:41 [PATCH] sh: Enable PMB support on SH4AL-DSP Paul Mundt
                   ` (3 preceding siblings ...)
  2011-03-01  8:40 ` Magnus Damm
@ 2011-03-01  9:02 ` Paul Mundt
  2011-03-01  9:12 ` Magnus Damm
                   ` (2 subsequent siblings)
  7 siblings, 0 replies; 9+ messages in thread
From: Paul Mundt @ 2011-03-01  9:02 UTC (permalink / raw)
  To: linux-sh

On Tue, Mar 01, 2011 at 05:40:41PM +0900, Magnus Damm wrote:
> On Tue, Mar 1, 2011 at 5:06 PM, Paul Mundt <lethal@linux-sh.org> wrote:
> > On Tue, Mar 01, 2011 at 05:02:17PM +0900, Magnus Damm wrote:
> >> sh7722 SH4AL-DSP
> >> sh7366 SH4AL-DSP
> >> sh7343 SH4AL-DSP + EXT
> >> sh7367 SH4AL-DSP + EXT + PMB + BOOT
> >> sh7377 SH4AL-DSP + EXT + PMB + BOOT
> >> sh7372 SH4AL-DSP + EXT + PMB + BOOT
> >>
> >> EXT = "SH4AL-DSP Extended Functions"
> >> PMB = "32-bit Address Extended Mode"
> >> BOOT = "32-bit Boot Function"
> >>
> >> From the table it looks like the SH4AL-DSP core can come with and without PMB.
> >>
> > That's a pretty special way of interpreting the results.
> >
> >> How would you like to special case it?
> >>
> > From the above table we see that all SH4AL-DSPs that are standalone cores
> > have no PMB functionality, and that the only ones that do are in ARM/SH
> > multi-core configurations. This is what needs to be special cased.
> 
> So the ARM/SH multi-core configurations with SH4A cores do not?
> 
I'm not sure how you extracted that question out of the above or even
what relevance it has to anything.

> > Implying that the potential existence of the PMB has anything at all to
> > do with SH4AL-DSP outside of the SH/R-Mobile context is disingenous at
> > best.
> > If a standalone SH4AL-DSP pops up with PMB functionality in the future
> > then of course that can be special cased too, but for now implying that
> > SH4AL-DSP = PMB is absurd.
> 
> I'm not implying that. Your logic seems to be inverse somehow. The
> patch does not imply "SH4AL-DSP = PMB", instead it simply removes the
> assumption "SH4AL-DSP != PMB". You don't want to allow people to
> enable PMB on SH4AL-DSP?
> 
No, I do not want to allow people to enable support for something that
doesn't exist. Standard SH4AL-DSP parts do not have a PMB, at all,
period. The only parts with PMBs are regular SH-4A parts and apparently
all of the SH/R-Mobile multi-core parts regardless of whether they're
using an SH-4A or SH4AL-DSP based SH core. This is what needs to be
special cased, and we're not going to pretend like the rest of the
SH4AL-DSP family can theoretically be equipped with PMB support just
because a special fork of the SoC line happened to introduce them without
reving the core version up.

> Take a look at sh7723. It has an X2 core and selects CPU_SH4A. People
> compiling for sh7723 can chose to enable CONFIG_PMB if they want to.
> This may not be the way you want it to work though, I'm not sure.
> 
All SH-4A parts support the PMB, so I'm not sure why this is surprising?

> But if the sh7723 case is like that then I can't see what is wrong
> with removing the !SH4AL_DSP bits for the Kconfig.
> 
Because SH4AL-DSP does not support the PMB. Regular SH-4A cores do, as do
apparently all of the R-Mobile parts regardless of which core was
included on the SH SoC side. This is where the special casing needs to
happen, and I'm not really sure why you seem to be having such a hard
time following the dependency chain.

There are certain things that can be inferred from each core family
revision. We know that SH-X has a certain subset of features, likewise
for SH-X2, SH-X3, etc. this is likewise true for SH-4A and SH4AL-DSP. The
PMB is part of the core features on SH-4A but has never been part of
SH4AL-DSP. If some hardware people somewhere have taken an SH4AL-DSP and
bolted on some extra logic to it and made that a standard subset for
those particular SoCs, then we need to special case the CPU family and
pile the dependencies on top of that. The pattern as such needs to be one
of R-Mobile implies PMB rather than SH4AL-DSP implies PMB, as the latter
is demonstrably false.

If there are other bits that we know all SH/R-Mobile multicores support,
then the simplest thing is just to add an R-Mobile family and treat that
the same way as SH-X3 or the others are handled. We won't be able to make
any SH-4A or SH4AL-DSP inferences from that, so that will still need to
be up to the specific CPU subtypes to work out for themselves.

While my expectations are pretty low, the PVR/PRR values should also be
consulted for all of the PMB-equipped SH/R-Mobile CPUs, as it is possible
that there's a capability bit that has been set to denote this, or that
the major rev has gone up so it's possible to do the family assignment
dynamically at probe time.

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] sh: Enable PMB support on SH4AL-DSP
  2011-03-01  6:41 [PATCH] sh: Enable PMB support on SH4AL-DSP Paul Mundt
                   ` (4 preceding siblings ...)
  2011-03-01  9:02 ` Paul Mundt
@ 2011-03-01  9:12 ` Magnus Damm
  2011-03-01  9:25 ` Paul Mundt
  2011-03-02  4:39 ` Magnus Damm
  7 siblings, 0 replies; 9+ messages in thread
From: Magnus Damm @ 2011-03-01  9:12 UTC (permalink / raw)
  To: linux-sh

On Tue, Mar 1, 2011 at 6:02 PM, Paul Mundt <lethal@linux-sh.org> wrote:
> On Tue, Mar 01, 2011 at 05:40:41PM +0900, Magnus Damm wrote:
>> On Tue, Mar 1, 2011 at 5:06 PM, Paul Mundt <lethal@linux-sh.org> wrote:
>> > On Tue, Mar 01, 2011 at 05:02:17PM +0900, Magnus Damm wrote:
>> >> sh7722 SH4AL-DSP
>> >> sh7366 SH4AL-DSP
>> >> sh7343 SH4AL-DSP + EXT
>> >> sh7367 SH4AL-DSP + EXT + PMB + BOOT
>> >> sh7377 SH4AL-DSP + EXT + PMB + BOOT
>> >> sh7372 SH4AL-DSP + EXT + PMB + BOOT
>> >>
>> >> EXT = "SH4AL-DSP Extended Functions"
>> >> PMB = "32-bit Address Extended Mode"
>> >> BOOT = "32-bit Boot Function"
>> >>
>> >> From the table it looks like the SH4AL-DSP core can come with and without PMB.
>> >>
>> > That's a pretty special way of interpreting the results.
>> >
>> >> How would you like to special case it?
>> >>
>> > From the above table we see that all SH4AL-DSPs that are standalone cores
>> > have no PMB functionality, and that the only ones that do are in ARM/SH
>> > multi-core configurations. This is what needs to be special cased.
>>
>> So the ARM/SH multi-core configurations with SH4A cores do not?
>>
> I'm not sure how you extracted that question out of the above or even
> what relevance it has to anything.

You seem to want to special case ARM/SH multi-core configurations. I
don't disagree with that, but you are arguing about enabling PMB on
SH4AL-DSP or not. Which I find rather strange, since the PMB may or
may not be included in SH4A.

So the ARM/SH multi-core processors with SH4A does not need this patch.

>> > Implying that the potential existence of the PMB has anything at all to
>> > do with SH4AL-DSP outside of the SH/R-Mobile context is disingenous at
>> > best.
>> > If a standalone SH4AL-DSP pops up with PMB functionality in the future
>> > then of course that can be special cased too, but for now implying that
>> > SH4AL-DSP = PMB is absurd.
>>
>> I'm not implying that. Your logic seems to be inverse somehow. The
>> patch does not imply "SH4AL-DSP = PMB", instead it simply removes the
>> assumption "SH4AL-DSP != PMB". You don't want to allow people to
>> enable PMB on SH4AL-DSP?
>>
> No, I do not want to allow people to enable support for something that
> doesn't exist. Standard SH4AL-DSP parts do not have a PMB, at all,
> period. The only parts with PMBs are regular SH-4A parts and apparently
> all of the SH/R-Mobile multi-core parts regardless of whether they're
> using an SH-4A or SH4AL-DSP based SH core. This is what needs to be
> special cased, and we're not going to pretend like the rest of the
> SH4AL-DSP family can theoretically be equipped with PMB support just
> because a special fork of the SoC line happened to introduce them without
> reving the core version up.

And you're certain that you've seen all SH4AL-DSP parts available?

>> Take a look at sh7723. It has an X2 core and selects CPU_SH4A. People
>> compiling for sh7723 can chose to enable CONFIG_PMB if they want to.
>> This may not be the way you want it to work though, I'm not sure.
>>
> All SH-4A parts support the PMB, so I'm not sure why this is surprising?

sh7723 has an SH4A yes, but no PMB support is included. At least
that's what the data sheet says.

>> But if the sh7723 case is like that then I can't see what is wrong
>> with removing the !SH4AL_DSP bits for the Kconfig.
>>
> Because SH4AL-DSP does not support the PMB. Regular SH-4A cores do, as do
> apparently all of the R-Mobile parts regardless of which core was
> included on the SH SoC side. This is where the special casing needs to
> happen, and I'm not really sure why you seem to be having such a hard
> time following the dependency chain.

I believe that the PMB feature can be added to SH4A or SH4A-DSP,
regardless of if it's included in an ARM multi-core package.

> There are certain things that can be inferred from each core family
> revision. We know that SH-X has a certain subset of features, likewise
> for SH-X2, SH-X3, etc. this is likewise true for SH-4A and SH4AL-DSP. The
> PMB is part of the core features on SH-4A but has never been part of
> SH4AL-DSP. If some hardware people somewhere have taken an SH4AL-DSP and
> bolted on some extra logic to it and made that a standard subset for
> those particular SoCs, then we need to special case the CPU family and
> pile the dependencies on top of that. The pattern as such needs to be one
> of R-Mobile implies PMB rather than SH4AL-DSP implies PMB, as the latter
> is demonstrably false.

But sh7723 is SH-Mobile but it does not support PMB.

> If there are other bits that we know all SH/R-Mobile multicores support,
> then the simplest thing is just to add an R-Mobile family and treat that
> the same way as SH-X3 or the others are handled. We won't be able to make
> any SH-4A or SH4AL-DSP inferences from that, so that will still need to
> be up to the specific CPU subtypes to work out for themselves.
>
> While my expectations are pretty low, the PVR/PRR values should also be
> consulted for all of the PMB-equipped SH/R-Mobile CPUs, as it is possible
> that there's a capability bit that has been set to denote this, or that
> the major rev has gone up so it's possible to do the family assignment
> dynamically at probe time.

That's worth investigating, yes.

/ magnus

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] sh: Enable PMB support on SH4AL-DSP
  2011-03-01  6:41 [PATCH] sh: Enable PMB support on SH4AL-DSP Paul Mundt
                   ` (5 preceding siblings ...)
  2011-03-01  9:12 ` Magnus Damm
@ 2011-03-01  9:25 ` Paul Mundt
  2011-03-02  4:39 ` Magnus Damm
  7 siblings, 0 replies; 9+ messages in thread
From: Paul Mundt @ 2011-03-01  9:25 UTC (permalink / raw)
  To: linux-sh

On Tue, Mar 01, 2011 at 06:12:25PM +0900, Magnus Damm wrote:
> On Tue, Mar 1, 2011 at 6:02 PM, Paul Mundt <lethal@linux-sh.org> wrote:
> > I'm not sure how you extracted that question out of the above or even
> > what relevance it has to anything.
> 
> You seem to want to special case ARM/SH multi-core configurations. I
> don't disagree with that, but you are arguing about enabling PMB on
> SH4AL-DSP or not. Which I find rather strange, since the PMB may or
> may not be included in SH4A.
> 
To date PMB support has existed in every SH-4A, and has never existed in
any SH4AL-DSP. How much more straightforward do you want me to spell it
out for you?

> > No, I do not want to allow people to enable support for something that
> > doesn't exist. Standard SH4AL-DSP parts do not have a PMB, at all,
> > period. The only parts with PMBs are regular SH-4A parts and apparently
> > all of the SH/R-Mobile multi-core parts regardless of whether they're
> > using an SH-4A or SH4AL-DSP based SH core. This is what needs to be
> > special cased, and we're not going to pretend like the rest of the
> > SH4AL-DSP family can theoretically be equipped with PMB support just
> > because a special fork of the SoC line happened to introduce them without
> > reving the core version up.
> 
> And you're certain that you've seen all SH4AL-DSP parts available?
> 
We do not make configuration options based on arbitrary speculation, only
on facts. To date there have been no SH4AL-DSP parts with a PMB as part
of its configuration. If and when one shows up, it gets special cased.

> >> Take a look at sh7723. It has an X2 core and selects CPU_SH4A. People
> >> compiling for sh7723 can chose to enable CONFIG_PMB if they want to.
> >> This may not be the way you want it to work though, I'm not sure.
> >>
> > All SH-4A parts support the PMB, so I'm not sure why this is surprising?
> 
> sh7723 has an SH4A yes, but no PMB support is included. At least
> that's what the data sheet says.
> 
Then SH7723 violates SH-4A specifications and likewise needs to have its
dependencies tightened down.

> >> But if the sh7723 case is like that then I can't see what is wrong
> >> with removing the !SH4AL_DSP bits for the Kconfig.
> >>
> > Because SH4AL-DSP does not support the PMB. Regular SH-4A cores do, as do
> > apparently all of the R-Mobile parts regardless of which core was
> > included on the SH SoC side. This is where the special casing needs to
> > happen, and I'm not really sure why you seem to be having such a hard
> > time following the dependency chain.
> 
> I believe that the PMB feature can be added to SH4A or SH4A-DSP,
> regardless of if it's included in an ARM multi-core package.
> 
Fortunately what you believe has no bearing on the issue at hand. While
any IP block can be added to any CPU subtype in theory, we don't make
arbitrary combinations available up until the point where a certain
configuration is demonstrable for a given subtype.

Show me the hardware and its PRR/PVR settings, and then we can work out
how best to express that in Kconfig language. Arbitrary hand-waving is
not how we do things, particularly when it comes to options that make
randomized configurations unbootable.

> > There are certain things that can be inferred from each core family
> > revision. We know that SH-X has a certain subset of features, likewise
> > for SH-X2, SH-X3, etc. this is likewise true for SH-4A and SH4AL-DSP. The
> > PMB is part of the core features on SH-4A but has never been part of
> > SH4AL-DSP. If some hardware people somewhere have taken an SH4AL-DSP and
> > bolted on some extra logic to it and made that a standard subset for
> > those particular SoCs, then we need to special case the CPU family and
> > pile the dependencies on top of that. The pattern as such needs to be one
> > of R-Mobile implies PMB rather than SH4AL-DSP implies PMB, as the latter
> > is demonstrably false.
> 
> But sh7723 is SH-Mobile but it does not support PMB.
> 
Then this is a bug and needs to be corrected, not blindly layered on top
of.

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] sh: Enable PMB support on SH4AL-DSP
  2011-03-01  6:41 [PATCH] sh: Enable PMB support on SH4AL-DSP Paul Mundt
                   ` (6 preceding siblings ...)
  2011-03-01  9:25 ` Paul Mundt
@ 2011-03-02  4:39 ` Magnus Damm
  7 siblings, 0 replies; 9+ messages in thread
From: Magnus Damm @ 2011-03-02  4:39 UTC (permalink / raw)
  To: linux-sh

On Tue, Mar 1, 2011 at 6:25 PM, Paul Mundt <lethal@linux-sh.org> wrote:
> On Tue, Mar 01, 2011 at 06:12:25PM +0900, Magnus Damm wrote:
>> On Tue, Mar 1, 2011 at 6:02 PM, Paul Mundt <lethal@linux-sh.org> wrote:
>> > I'm not sure how you extracted that question out of the above or even
>> > what relevance it has to anything.
>>
>> You seem to want to special case ARM/SH multi-core configurations. I
>> don't disagree with that, but you are arguing about enabling PMB on
>> SH4AL-DSP or not. Which I find rather strange, since the PMB may or
>> may not be included in SH4A.
>>
> To date PMB support has existed in every SH-4A, and has never existed in
> any SH4AL-DSP. How much more straightforward do you want me to spell it
> out for you?

I suppose you base this on the "SH-4A Software Manual" and the
"SH4AL-DSP Software Manual". The PMB is included in the SH-4A document
but excluded in the SH4AL-DSP doc. That seems to match with your
claims.

The actual data sheet for each individual processor tells us this:

SH-4A with PMB:
sh7785, sh7780, sh7764

SH-4A without PMB:
sh7730*, sh7723*

SH4AL-DSP with PMB:
sh7367, sh7377, sh7372

SHAL-DSP without PMB:
sh7366**, sh7343**, sh7722**

* Not clearly specified as SH-4A. The instruction set is described as
SH-4A at least.
** Clearly specified as SH4AL-DSP with PMB in the data sheet.

>> > No, I do not want to allow people to enable support for something that
>> > doesn't exist. Standard SH4AL-DSP parts do not have a PMB, at all,
>> > period. The only parts with PMBs are regular SH-4A parts and apparently
>> > all of the SH/R-Mobile multi-core parts regardless of whether they're
>> > using an SH-4A or SH4AL-DSP based SH core. This is what needs to be
>> > special cased, and we're not going to pretend like the rest of the
>> > SH4AL-DSP family can theoretically be equipped with PMB support just
>> > because a special fork of the SoC line happened to introduce them without
>> > reving the core version up.
>>
>> And you're certain that you've seen all SH4AL-DSP parts available?
>>
> We do not make configuration options based on arbitrary speculation, only
> on facts. To date there have been no SH4AL-DSP parts with a PMB as part
> of its configuration. If and when one shows up, it gets special cased.

You've got a list of SH4AL-DSP parts with PMB now.

>> >> Take a look at sh7723. It has an X2 core and selects CPU_SH4A. People
>> >> compiling for sh7723 can chose to enable CONFIG_PMB if they want to.
>> >> This may not be the way you want it to work though, I'm not sure.
>> >>
>> > All SH-4A parts support the PMB, so I'm not sure why this is surprising?
>>
>> sh7723 has an SH4A yes, but no PMB support is included. At least
>> that's what the data sheet says.
>>
> Then SH7723 violates SH-4A specifications and likewise needs to have its
> dependencies tightened down.

Sure.

>> >> But if the sh7723 case is like that then I can't see what is wrong
>> >> with removing the !SH4AL_DSP bits for the Kconfig.
>> >>
>> > Because SH4AL-DSP does not support the PMB. Regular SH-4A cores do, as do
>> > apparently all of the R-Mobile parts regardless of which core was
>> > included on the SH SoC side. This is where the special casing needs to
>> > happen, and I'm not really sure why you seem to be having such a hard
>> > time following the dependency chain.
>>
>> I believe that the PMB feature can be added to SH4A or SH4A-DSP,
>> regardless of if it's included in an ARM multi-core package.
>>
> Fortunately what you believe has no bearing on the issue at hand. While
> any IP block can be added to any CPU subtype in theory, we don't make
> arbitrary combinations available up until the point where a certain
> configuration is demonstrable for a given subtype.
>
> Show me the hardware and its PRR/PVR settings, and then we can work out
> how best to express that in Kconfig language. Arbitrary hand-waving is
> not how we do things, particularly when it comes to options that make
> randomized configurations unbootable.

The SH4AL-DSP core included in sh7372 gives this:

PVR\x10300b00 CVRC440110 PRR\0002d00

>> > There are certain things that can be inferred from each core family
>> > revision. We know that SH-X has a certain subset of features, likewise
>> > for SH-X2, SH-X3, etc. this is likewise true for SH-4A and SH4AL-DSP. The
>> > PMB is part of the core features on SH-4A but has never been part of
>> > SH4AL-DSP. If some hardware people somewhere have taken an SH4AL-DSP and
>> > bolted on some extra logic to it and made that a standard subset for
>> > those particular SoCs, then we need to special case the CPU family and
>> > pile the dependencies on top of that. The pattern as such needs to be one
>> > of R-Mobile implies PMB rather than SH4AL-DSP implies PMB, as the latter
>> > is demonstrably false.
>>
>> But sh7723 is SH-Mobile but it does not support PMB.
>>
> Then this is a bug and needs to be corrected, not blindly layered on top
> of.

OK, I wasn't aware that it was a bug. I thought it was a feature that
PMB could be enabled or disabled in the kernel configuration
regardless of the actual CPU support. So you can chose to enable it in
your kernel if you'd like, and the actual detection happens during
runtime. But I suppose that isn't the way it works.

/ magnus

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2011-03-02  4:39 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-03-01  6:41 [PATCH] sh: Enable PMB support on SH4AL-DSP Paul Mundt
2011-03-01  6:45 ` Magnus Damm
2011-03-01  8:02 ` Magnus Damm
2011-03-01  8:06 ` Paul Mundt
2011-03-01  8:40 ` Magnus Damm
2011-03-01  9:02 ` Paul Mundt
2011-03-01  9:12 ` Magnus Damm
2011-03-01  9:25 ` Paul Mundt
2011-03-02  4:39 ` Magnus Damm

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).