[PATCH 08/11] of: Increase MAX_PHANDLE_ARGS

[PATCH 08/11] of: Increase MAX_PHANDLE_ARGS

[Andreas Herrmann]

arm-smmu driver uses of_parse_phandle_with_args when parsing DT
information to determine stream IDs for a master device.
Thus the number of stream IDs per master device is bound by
MAX_PHANDLE_ARGS.

To support Calxeda ECX-2000 hardware arm-smmu driver requires a
slightly higher value for MAX_PHANDLE_ARGS as this hardware has 10
stream IDs for one master device.

Cc: Grant Likely <grant.likely@linaro.org>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: devicetree@vger.kernel.org
Cc: Andreas Herrmann <herrmann.der.user@googlemail.com>
Signed-off-by: Andreas Herrmann <andreas.herrmann@calxeda.com>
---
 include/linux/of.h |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/linux/of.h b/include/linux/of.h
index 276c546..0807af8 100644
--- a/include/linux/of.h
+++ b/include/linux/of.h
@@ -67,7 +67,7 @@ struct device_node {
 #endif
 };
 
-#define MAX_PHANDLE_ARGS 8
+#define MAX_PHANDLE_ARGS 10
 struct of_phandle_args {
 	struct device_node *np;
 	int args_count;
-- 
1.7.9.5

Re: [PATCH 08/11] of: Increase MAX_PHANDLE_ARGS

[Rob Herring]

On Thu, Jan 16, 2014 at 6:44 AM, Andreas Herrmann
<andreas.herrmann-bsGFqQB8/DxBDgjK7y7TUQ@public.gmane.org> wrote:
> arm-smmu driver uses of_parse_phandle_with_args when parsing DT
> information to determine stream IDs for a master device.
> Thus the number of stream IDs per master device is bound by
> MAX_PHANDLE_ARGS.
>
> To support Calxeda ECX-2000 hardware arm-smmu driver requires a
> slightly higher value for MAX_PHANDLE_ARGS as this hardware has 10
> stream IDs for one master device.
>
> Cc: Grant Likely <grant.likely-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> Cc: Rob Herring <robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> Cc: devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> Cc: Andreas Herrmann <herrmann.der.user-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
> Signed-off-by: Andreas Herrmann <andreas.herrmann-bsGFqQB8/DxBDgjK7y7TUQ@public.gmane.org>

Acked-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>

One comment below...

> ---
>  include/linux/of.h |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/include/linux/of.h b/include/linux/of.h
> index 276c546..0807af8 100644
> --- a/include/linux/of.h
> +++ b/include/linux/of.h
> @@ -67,7 +67,7 @@ struct device_node {
>  #endif
>  };
>
> -#define MAX_PHANDLE_ARGS 8
> +#define MAX_PHANDLE_ARGS 10

Just bump this to 16. This is normally just a temporary on the stack
and 8 more words on the stack is not going to cost much.

Rob

Re: [PATCH 08/11] of: Increase MAX_PHANDLE_ARGS

[Andreas Herrmann]

On Thu, Jan 16, 2014 at 09:25:59AM -0500, Rob Herring wrote:
> On Thu, Jan 16, 2014 at 6:44 AM, Andreas Herrmann
> <andreas.herrmann-bsGFqQB8/DxBDgjK7y7TUQ@public.gmane.org> wrote:
> > arm-smmu driver uses of_parse_phandle_with_args when parsing DT
> > information to determine stream IDs for a master device.
> > Thus the number of stream IDs per master device is bound by
> > MAX_PHANDLE_ARGS.
> >
> > To support Calxeda ECX-2000 hardware arm-smmu driver requires a
> > slightly higher value for MAX_PHANDLE_ARGS as this hardware has 10
> > stream IDs for one master device.
> >
> > Cc: Grant Likely <grant.likely-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> > Cc: Rob Herring <robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> > Cc: devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> > Cc: Andreas Herrmann <herrmann.der.user-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
> > Signed-off-by: Andreas Herrmann <andreas.herrmann-bsGFqQB8/DxBDgjK7y7TUQ@public.gmane.org>
> 
> Acked-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> 
> One comment below...
> 
> > ---
> >  include/linux/of.h |    2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/include/linux/of.h b/include/linux/of.h
> > index 276c546..0807af8 100644
> > --- a/include/linux/of.h
> > +++ b/include/linux/of.h
> > @@ -67,7 +67,7 @@ struct device_node {
> >  #endif
> >  };
> >
> > -#define MAX_PHANDLE_ARGS 8
> > +#define MAX_PHANDLE_ARGS 10
> 
> Just bump this to 16. This is normally just a temporary on the stack
> and 8 more words on the stack is not going to cost much.

Yes, that seems reasonable.


Thanks,

Andreas

[PATCH v2 08/11] of: Increase MAX_PHANDLE_ARGS

[Andreas Herrmann]

arm-smmu driver uses of_parse_phandle_with_args when parsing DT
information to determine stream IDs for a master device.
Thus the number of stream IDs per master device is bound by
MAX_PHANDLE_ARGS.

To support Calxeda ECX-2000 hardware arm-smmu driver requires a
slightly higher value for MAX_PHANDLE_ARGS as this hardware has 10
stream IDs for one master device.

Increasing it to 16 seems a reasonable choice.

Cc: Grant Likely <grant.likely-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
Cc: Rob Herring <robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
Cc: devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Cc: Andreas Herrmann <herrmann.der.user-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
Acked-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
Signed-off-by: Andreas Herrmann <andreas.herrmann-bsGFqQB8/DxBDgjK7y7TUQ@public.gmane.org>
---
 include/linux/of.h |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/linux/of.h b/include/linux/of.h
index 276c546..24e1b28 100644
--- a/include/linux/of.h
+++ b/include/linux/of.h
@@ -67,7 +67,7 @@ struct device_node {
 #endif
 };
 
-#define MAX_PHANDLE_ARGS 8
+#define MAX_PHANDLE_ARGS 16
 struct of_phandle_args {
 	struct device_node *np;
 	int args_count;
-- 
1.7.9.5

Re: [PATCH v2 08/11] of: Increase MAX_PHANDLE_ARGS

[Suravee Suthikulanit]

On 1/17/2014 5:08 AM, Andreas Herrmann wrote:
>
> arm-smmu driver uses of_parse_phandle_with_args when parsing DT
> information to determine stream IDs for a master device.
> Thus the number of stream IDs per master device is bound by
> MAX_PHANDLE_ARGS.
>
> To support Calxeda ECX-2000 hardware arm-smmu driver requires a
> slightly higher value for MAX_PHANDLE_ARGS as this hardware has 10
> stream IDs for one master device.
>
> Increasing it to 16 seems a reasonable choice.
>
> Cc: Grant Likely <grant.likely-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> Cc: Rob Herring <robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> Cc: devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> Cc: Andreas Herrmann <herrmann.der.user-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
> Acked-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> Signed-off-by: Andreas Herrmann <andreas.herrmann-bsGFqQB8/DxBDgjK7y7TUQ@public.gmane.org>
> ---
>   include/linux/of.h |    2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/include/linux/of.h b/include/linux/of.h
> index 276c546..24e1b28 100644
> --- a/include/linux/of.h
> +++ b/include/linux/of.h
> @@ -67,7 +67,7 @@ struct device_node {
>   #endif
>   };
>
> -#define MAX_PHANDLE_ARGS 8
> +#define MAX_PHANDLE_ARGS 16

Since the MMU-500 specify "Number of SMRs" upto 128 registers, shouldn't 
this be changed to be able to support 128 StreamIDs as well?  Although I 
am not sure if this would be too big to have on the stack per Rob's 
comment in the previous patch set.

Thank you,

Suravee

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 08/11] of: Increase MAX_PHANDLE_ARGS

[Rob Herring]

On Wed, Jan 29, 2014 at 10:11 AM, Suravee Suthikulanit
<suravee.suthikulpanit-5C7GfCeVMHo@public.gmane.org> wrote:
> On 1/17/2014 5:08 AM, Andreas Herrmann wrote:
>>
>>
>> arm-smmu driver uses of_parse_phandle_with_args when parsing DT
>> information to determine stream IDs for a master device.
>> Thus the number of stream IDs per master device is bound by
>> MAX_PHANDLE_ARGS.
>>
>> To support Calxeda ECX-2000 hardware arm-smmu driver requires a
>> slightly higher value for MAX_PHANDLE_ARGS as this hardware has 10
>> stream IDs for one master device.
>>
>> Increasing it to 16 seems a reasonable choice.
>>
>> Cc: Grant Likely <grant.likely-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
>> Cc: Rob Herring <robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
>> Cc: devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
>> Cc: Andreas Herrmann <herrmann.der.user-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
>> Acked-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
>> Signed-off-by: Andreas Herrmann <andreas.herrmann-bsGFqQB8/DxBDgjK7y7TUQ@public.gmane.org>
>> ---
>>   include/linux/of.h |    2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/include/linux/of.h b/include/linux/of.h
>> index 276c546..24e1b28 100644
>> --- a/include/linux/of.h
>> +++ b/include/linux/of.h
>> @@ -67,7 +67,7 @@ struct device_node {
>>   #endif
>>   };
>>
>> -#define MAX_PHANDLE_ARGS 8
>> +#define MAX_PHANDLE_ARGS 16
>
>
> Since the MMU-500 specify "Number of SMRs" upto 128 registers, shouldn't
> this be changed to be able to support 128 StreamIDs as well?  Although I am
> not sure if this would be too big to have on the stack per Rob's comment in
> the previous patch set.

Do you actually need 128 now? If not, then we can deal with that when
we get there. There are lots of things in spec's that are not actually
implemented or supported.

Rob

Re: [PATCH v2 08/11] of: Increase MAX_PHANDLE_ARGS

[Suravee Suthikulanit]

On 1/29/2014 10:57 AM, Rob Herring wrote:
>>> diff --git a/include/linux/of.h b/include/linux/of.h
>>> >>index 276c546..24e1b28 100644
>>> >>--- a/include/linux/of.h
>>> >>+++ b/include/linux/of.h
>>> >>@@ -67,7 +67,7 @@ struct device_node {
>>> >>   #endif
>>> >>   };
>>> >>
>>> >>-#define MAX_PHANDLE_ARGS 8
>>> >>+#define MAX_PHANDLE_ARGS 16
>> >
>> >
>> >Since the MMU-500 specify "Number of SMRs" upto 128 registers, shouldn't
>> >this be changed to be able to support 128 StreamIDs as well?  Although I am
>> >not sure if this would be too big to have on the stack per Rob's comment in
>> >the previous patch set.
> Do you actually need 128 now? If not, then we can deal with that when
> we get there. There are lots of things in spec's that are not actually
> implemented or supported.

Actually, we are using 32 on the AMD system. So, do you think we can set 
this to 32 instead?

Thanks,

Suravee

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 08/11] of: Increase MAX_PHANDLE_ARGS

[Andreas Herrmann]

On Wed, Jan 29, 2014 at 11:59:12AM -0500, Suravee Suthikulanit wrote:
> On 1/29/2014 10:57 AM, Rob Herring wrote:
> >>> diff --git a/include/linux/of.h b/include/linux/of.h
> >>> >>index 276c546..24e1b28 100644
> >>> >>--- a/include/linux/of.h
> >>> >>+++ b/include/linux/of.h
> >>> >>@@ -67,7 +67,7 @@ struct device_node {
> >>> >>   #endif
> >>> >>   };
> >>> >>
> >>> >>-#define MAX_PHANDLE_ARGS 8
> >>> >>+#define MAX_PHANDLE_ARGS 16
> >> >
> >> >
> >> >Since the MMU-500 specify "Number of SMRs" upto 128 registers, shouldn't
> >> >this be changed to be able to support 128 StreamIDs as well?  Although I am
> >> >not sure if this would be too big to have on the stack per Rob's comment in
> >> >the previous patch set.
> > Do you actually need 128 now? If not, then we can deal with that when
> > we get there. There are lots of things in spec's that are not actually
> > implemented or supported.
> 
> Actually, we are using 32 on the AMD system. So, do you think we can set 
> this to 32 instead?

I think that's ok.

But are we really talking about number of SMRs or number of StreamIDs
per master device here? Ie. are you just having 32 SMRs for an SMMU on
your AMD system or do you have master devices which have 32 StreamIDs?

If it's just number of SMRs we don't need to modify this macro.


Andreas

Re: [PATCH v2 08/11] of: Increase MAX_PHANDLE_ARGS

[Suravee Suthikulanit]

On 1/29/2014 11:16 AM, Andreas Herrmann wrote:
> On Wed, Jan 29, 2014 at 11:59:12AM -0500, Suravee Suthikulanit wrote:
>> On 1/29/2014 10:57 AM, Rob Herring wrote:
>>>>> diff --git a/include/linux/of.h b/include/linux/of.h
>>>>>>> index 276c546..24e1b28 100644
>>>>>>> --- a/include/linux/of.h
>>>>>>> +++ b/include/linux/of.h
>>>>>>> @@ -67,7 +67,7 @@ struct device_node {
>>>>>>>    #endif
>>>>>>>    };
>>>>>>>
>>>>>>> -#define MAX_PHANDLE_ARGS 8
>>>>>>> +#define MAX_PHANDLE_ARGS 16
>>>>>
>>>>>
>>>>> Since the MMU-500 specify "Number of SMRs" upto 128 registers, shouldn't
>>>>> this be changed to be able to support 128 StreamIDs as well?  Although I am
>>>>> not sure if this would be too big to have on the stack per Rob's comment in
>>>>> the previous patch set.
>>> Do you actually need 128 now? If not, then we can deal with that when
>>> we get there. There are lots of things in spec's that are not actually
>>> implemented or supported.
>>
>> Actually, we are using 32 on the AMD system. So, do you think we can set
>> this to 32 instead?
>
> I think that's ok.
>
> But are we really talking about number of SMRs or number of StreamIDs
> per master device here? Ie. are you just having 32 SMRs for an SMMU on
> your AMD system or do you have master devices which have 32 StreamIDs?
>
> If it's just number of SMRs we don't need to modify this macro.
>
>
> Andreas
>

I am referring to the case where each mmu-master can have upto 32 streamID.

Suravee

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 08/11] of: Increase MAX_PHANDLE_ARGS

[Will Deacon]

On Wed, Jan 29, 2014 at 05:26:35PM +0000, Suravee Suthikulanit wrote:
> On 1/29/2014 11:16 AM, Andreas Herrmann wrote:
> > On Wed, Jan 29, 2014 at 11:59:12AM -0500, Suravee Suthikulanit wrote:
> >> Actually, we are using 32 on the AMD system. So, do you think we can set
> >> this to 32 instead?
> >
> > I think that's ok.
> >
> > But are we really talking about number of SMRs or number of StreamIDs
> > per master device here? Ie. are you just having 32 SMRs for an SMMU on
> > your AMD system or do you have master devices which have 32 StreamIDs?
> >
> > If it's just number of SMRs we don't need to modify this macro.
> >
> 
> I am referring to the case where each mmu-master can have upto 32 streamID.

Crikey, how many SMRs do you have? Andreas and I have been struggling to
write a decent allocator for those, so if you have any algorithms that don't
require a quantum computer, we'd love to hear from you :)!

Will

Re: [PATCH v2 08/11] of: Increase MAX_PHANDLE_ARGS

[Suravee Suthikulanit]

On 1/29/2014 11:29 AM, Will Deacon wrote:
> On Wed, Jan 29, 2014 at 05:26:35PM +0000, Suravee Suthikulanit wrote:
>> On 1/29/2014 11:16 AM, Andreas Herrmann wrote:
>>> On Wed, Jan 29, 2014 at 11:59:12AM -0500, Suravee Suthikulanit wrote:
>>>> Actually, we are using 32 on the AMD system. So, do you think we can set
>>>> this to 32 instead?
>>>
>>> I think that's ok.
>>>
>>> But are we really talking about number of SMRs or number of StreamIDs
>>> per master device here? Ie. are you just having 32 SMRs for an SMMU on
>>> your AMD system or do you have master devices which have 32 StreamIDs?
>>>
>>> If it's just number of SMRs we don't need to modify this macro.
>>>
>>
>> I am referring to the case where each mmu-master can have upto 32 streamID.
>
> Crikey, how many SMRs do you have? Andreas and I have been struggling to
> write a decent allocator for those, so if you have any algorithms that don't
> require a quantum computer, we'd love to hear from you :)!
>
> Will
>

Are you talking about the __arm_smmu_alloc_bitmap()?

Currently, we have configured the each SMMU to have 32 SMRs and using 
15-bit streamID. However, we mostly have upto 32 streamID for each 
master, and most of the SMMU only have one master.  So it looks like the 
current logic should be ok.

Suravee

Re: [PATCH v2 08/11] of: Increase MAX_PHANDLE_ARGS

[Will Deacon]

On Wed, Jan 29, 2014 at 05:57:16PM +0000, Suravee Suthikulanit wrote:
> On 1/29/2014 11:29 AM, Will Deacon wrote:
> > On Wed, Jan 29, 2014 at 05:26:35PM +0000, Suravee Suthikulanit wrote:
> >> On 1/29/2014 11:16 AM, Andreas Herrmann wrote:
> >>> On Wed, Jan 29, 2014 at 11:59:12AM -0500, Suravee Suthikulanit wrote:
> >>>> Actually, we are using 32 on the AMD system. So, do you think we can set
> >>>> this to 32 instead?
> >>>
> >>> I think that's ok.
> >>>
> >>> But are we really talking about number of SMRs or number of StreamIDs
> >>> per master device here? Ie. are you just having 32 SMRs for an SMMU on
> >>> your AMD system or do you have master devices which have 32 StreamIDs?
> >>>
> >>> If it's just number of SMRs we don't need to modify this macro.
> >>>
> >>
> >> I am referring to the case where each mmu-master can have upto 32 streamID.
> >
> > Crikey, how many SMRs do you have? Andreas and I have been struggling to
> > write a decent allocator for those, so if you have any algorithms that don't
> > require a quantum computer, we'd love to hear from you :)!
> >
> > Will
> >
> 
> Are you talking about the __arm_smmu_alloc_bitmap()?
> 
> Currently, we have configured the each SMMU to have 32 SMRs and using 
> 15-bit streamID. However, we mostly have upto 32 streamID for each 
> master, and most of the SMMU only have one master.  So it looks like the 
> current logic should be ok.

Interesting... how does that work for PCI? Do you force all devices behind a
given RC into the same address space?

Will

Re: [PATCH v2 08/11] of: Increase MAX_PHANDLE_ARGS

[Suravee Suthikulanit]

On 1/29/2014 12:03 PM, Will Deacon wrote:
> On Wed, Jan 29, 2014 at 05:57:16PM +0000, Suravee Suthikulanit wrote:
>> On 1/29/2014 11:29 AM, Will Deacon wrote:
>>> On Wed, Jan 29, 2014 at 05:26:35PM +0000, Suravee Suthikulanit wrote:
>>>> On 1/29/2014 11:16 AM, Andreas Herrmann wrote:
>>>>> On Wed, Jan 29, 2014 at 11:59:12AM -0500, Suravee Suthikulanit wrote:
>>>>>> Actually, we are using 32 on the AMD system. So, do you think we can set
>>>>>> this to 32 instead?
>>>>>
>>>>> I think that's ok.
>>>>>
>>>>> But are we really talking about number of SMRs or number of StreamIDs
>>>>> per master device here? Ie. are you just having 32 SMRs for an SMMU on
>>>>> your AMD system or do you have master devices which have 32 StreamIDs?
>>>>>
>>>>> If it's just number of SMRs we don't need to modify this macro.
>>>>>
>>>>
>>>> I am referring to the case where each mmu-master can have upto 32 streamID.
>>>
>>> Crikey, how many SMRs do you have? Andreas and I have been struggling to
>>> write a decent allocator for those, so if you have any algorithms that don't
>>> require a quantum computer, we'd love to hear from you :)!
>>>
>>> Will
>>>
>>
>> Are you talking about the __arm_smmu_alloc_bitmap()?
>>
>> Currently, we have configured the each SMMU to have 32 SMRs and using
>> 15-bit streamID. However, we mostly have upto 32 streamID for each
>> master, and most of the SMMU only have one master.  So it looks like the
>> current logic should be ok.
>
> Interesting... how does that work for PCI? Do you force all devices behind a
> given RC into the same address space?
>
> Will
>

For PCI devices, we are using the bus, device, and function id to make 
up the 15-bit SID for devices behind a particular PCI root complex.

I also notice that we are currently not supporting the streamID mask in 
the SMR.  Is this something planed for the future?

Suravee

Re: [PATCH v2 08/11] of: Increase MAX_PHANDLE_ARGS

[Will Deacon]

On Thu, Jan 30, 2014 at 10:53:35PM +0000, Suravee Suthikulanit wrote:
> On 1/29/2014 12:03 PM, Will Deacon wrote:
> > Interesting... how does that work for PCI? Do you force all devices behind a
> > given RC into the same address space?
> >
> 
> For PCI devices, we are using the bus, device, and function id to make 
> up the 15-bit SID for devices behind a particular PCI root complex.

Very good!

> I also notice that we are currently not supporting the streamID mask in 
> the SMR.  Is this something planed for the future?

Andreas and I are curently working on this -- that's what I was referring to
above. Any feedback from you would be welcomed.

Will

Re: [PATCH v2 08/11] of: Increase MAX_PHANDLE_ARGS

[Andreas Herrmann]

On Wed, Jan 29, 2014 at 12:26:35PM -0500, Suravee Suthikulanit wrote:
> On 1/29/2014 11:16 AM, Andreas Herrmann wrote:
> > On Wed, Jan 29, 2014 at 11:59:12AM -0500, Suravee Suthikulanit wrote:
> >> On 1/29/2014 10:57 AM, Rob Herring wrote:
> >>>>> diff --git a/include/linux/of.h b/include/linux/of.h
> >>>>>>> index 276c546..24e1b28 100644
> >>>>>>> --- a/include/linux/of.h
> >>>>>>> +++ b/include/linux/of.h
> >>>>>>> @@ -67,7 +67,7 @@ struct device_node {
> >>>>>>>    #endif
> >>>>>>>    };
> >>>>>>>
> >>>>>>> -#define MAX_PHANDLE_ARGS 8
> >>>>>>> +#define MAX_PHANDLE_ARGS 16
> >>>>>
> >>>>>
> >>>>> Since the MMU-500 specify "Number of SMRs" upto 128 registers, shouldn't
> >>>>> this be changed to be able to support 128 StreamIDs as well?  Although I am
> >>>>> not sure if this would be too big to have on the stack per Rob's comment in
> >>>>> the previous patch set.
> >>> Do you actually need 128 now? If not, then we can deal with that when
> >>> we get there. There are lots of things in spec's that are not actually
> >>> implemented or supported.
> >>
> >> Actually, we are using 32 on the AMD system. So, do you think we can set
> >> this to 32 instead?
> >
> > I think that's ok.
> >
> > But are we really talking about number of SMRs or number of StreamIDs
> > per master device here? Ie. are you just having 32 SMRs for an SMMU on
> > your AMD system or do you have master devices which have 32 StreamIDs?
> >
> > If it's just number of SMRs we don't need to modify this macro.

> I am referring to the case where each mmu-master can have upto 32 streamID.

Rob,

Do you agree on increasing MAX_PHANDLE_ARGS to 32?

Or should this be done when someone (e.g. Suravee) submits a DTS
update with an SMMU node description containing more than 16 stream
IDs for a master device?


Andreas

Re: [PATCH v2 08/11] of: Increase MAX_PHANDLE_ARGS

[Rob Herring]

On Thu, Jan 30, 2014 at 11:45 AM, Andreas Herrmann
<andreas.herrmann-bsGFqQB8/DxBDgjK7y7TUQ@public.gmane.org> wrote:
> On Wed, Jan 29, 2014 at 12:26:35PM -0500, Suravee Suthikulanit wrote:
>> On 1/29/2014 11:16 AM, Andreas Herrmann wrote:
>> > On Wed, Jan 29, 2014 at 11:59:12AM -0500, Suravee Suthikulanit wrote:
>> >> On 1/29/2014 10:57 AM, Rob Herring wrote:
>> >>>>> diff --git a/include/linux/of.h b/include/linux/of.h
>> >>>>>>> index 276c546..24e1b28 100644
>> >>>>>>> --- a/include/linux/of.h
>> >>>>>>> +++ b/include/linux/of.h
>> >>>>>>> @@ -67,7 +67,7 @@ struct device_node {
>> >>>>>>>    #endif
>> >>>>>>>    };
>> >>>>>>>
>> >>>>>>> -#define MAX_PHANDLE_ARGS 8
>> >>>>>>> +#define MAX_PHANDLE_ARGS 16
>> >>>>>
>> >>>>>
>> >>>>> Since the MMU-500 specify "Number of SMRs" upto 128 registers, shouldn't
>> >>>>> this be changed to be able to support 128 StreamIDs as well?  Although I am
>> >>>>> not sure if this would be too big to have on the stack per Rob's comment in
>> >>>>> the previous patch set.
>> >>> Do you actually need 128 now? If not, then we can deal with that when
>> >>> we get there. There are lots of things in spec's that are not actually
>> >>> implemented or supported.
>> >>
>> >> Actually, we are using 32 on the AMD system. So, do you think we can set
>> >> this to 32 instead?
>> >
>> > I think that's ok.
>> >
>> > But are we really talking about number of SMRs or number of StreamIDs
>> > per master device here? Ie. are you just having 32 SMRs for an SMMU on
>> > your AMD system or do you have master devices which have 32 StreamIDs?
>> >
>> > If it's just number of SMRs we don't need to modify this macro.
>
>> I am referring to the case where each mmu-master can have upto 32 streamID.
>
> Rob,
>
> Do you agree on increasing MAX_PHANDLE_ARGS to 32?

Yes, but more than that will require a closer look. Please get this
into next early in the cycle.

> Or should this be done when someone (e.g. Suravee) submits a DTS
> update with an SMMU node description containing more than 16 stream
> IDs for a master device?

Well, I am inclined to not care having seen no upstream activity for
AMD's platform.

Rob

Re: [PATCH v2 08/11] of: Increase MAX_PHANDLE_ARGS

[Will Deacon]

On Fri, Jan 31, 2014 at 04:24:09PM +0000, Rob Herring wrote:
> On Thu, Jan 30, 2014 at 11:45 AM, Andreas Herrmann
> <andreas.herrmann-bsGFqQB8/DxBDgjK7y7TUQ@public.gmane.org> wrote:
> > Do you agree on increasing MAX_PHANDLE_ARGS to 32?
> 
> Yes, but more than that will require a closer look. Please get this
> into next early in the cycle.

If you want to get this patch in early, I suggest taking it via a tree other
than mine. I don't put my iommu queue into -next, since it goes to Joerg
around -rc4, who then sticks it in after that.

Will

Re: [PATCH v2 08/11] of: Increase MAX_PHANDLE_ARGS

[Grant Likely]

On Wed, 29 Jan 2014 10:59:12 -0600, Suravee Suthikulanit <suravee.suthikulpanit-5C7GfCeVMHo@public.gmane.org> wrote:
> On 1/29/2014 10:57 AM, Rob Herring wrote:
> >>> diff --git a/include/linux/of.h b/include/linux/of.h
> >>> >>index 276c546..24e1b28 100644
> >>> >>--- a/include/linux/of.h
> >>> >>+++ b/include/linux/of.h
> >>> >>@@ -67,7 +67,7 @@ struct device_node {
> >>> >>   #endif
> >>> >>   };
> >>> >>
> >>> >>-#define MAX_PHANDLE_ARGS 8
> >>> >>+#define MAX_PHANDLE_ARGS 16
> >> >
> >> >
> >> >Since the MMU-500 specify "Number of SMRs" upto 128 registers, shouldn't
> >> >this be changed to be able to support 128 StreamIDs as well?  Although I am
> >> >not sure if this would be too big to have on the stack per Rob's comment in
> >> >the previous patch set.
> > Do you actually need 128 now? If not, then we can deal with that when
> > we get there. There are lots of things in spec's that are not actually
> > implemented or supported.
> 
> Actually, we are using 32 on the AMD system. So, do you think we can set 
> this to 32 instead?

The helper really wasn't designed for large number of arguments to a
phandle. If the phandle args are really that large, then it may be
better to have a parser that allocates the space needed and/or puts the
data directly into the destination data structure.

g.

Re: [PATCH v2 08/11] of: Increase MAX_PHANDLE_ARGS

[Grant Likely]

On Fri, 17 Jan 2014 12:08:30 +0100, Andreas Herrmann <andreas.herrmann-bsGFqQB8/DxBDgjK7y7TUQ@public.gmane.org> wrote:
> 
> arm-smmu driver uses of_parse_phandle_with_args when parsing DT
> information to determine stream IDs for a master device.
> Thus the number of stream IDs per master device is bound by
> MAX_PHANDLE_ARGS.
> 
> To support Calxeda ECX-2000 hardware arm-smmu driver requires a
> slightly higher value for MAX_PHANDLE_ARGS as this hardware has 10
> stream IDs for one master device.
> 
> Increasing it to 16 seems a reasonable choice.
> 
> Cc: Grant Likely <grant.likely-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> Cc: Rob Herring <robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> Cc: devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> Cc: Andreas Herrmann <herrmann.der.user-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
> Acked-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> Signed-off-by: Andreas Herrmann <andreas.herrmann-bsGFqQB8/DxBDgjK7y7TUQ@public.gmane.org>

I've merged this one, but I'm not excited about making it any larger
because this structure lives on the stack most of the time.

g.