Re: OpenGL vs OpenGlES on arm64

2018-11-26 Thread Rob Herring
On Mon, Nov 26, 2018 at 9:10 AM Tom Gall  wrote:
>
> On Mon, Nov 26, 2018 at 8:46 AM Steve McIntyre
>  wrote:
> >
> > On Mon, Nov 26, 2018 at 02:11:10PM +, Graeme Gregory wrote:
> > >On Mon, 26 Nov 2018 at 14:03, Tom Gall  wrote:
> > >>
> > >> Hi Wookey,
> > >>
> > >> This was something bouncing around in the Graphics Working Group back
> > >> in the day. Alexandros as I recall was the key dev. As far as a shim
> > >> goes given the effort that would have been involved and the lack of
> > >> interest, it wasn't worked on.
> > >>
> > >> For ARM64 and QT would a move to GLES be a "good thing?"  Yes.
> > >>
> > >> When it comes to graphics drivers today for arm hardware GLES is
> > >> pretty universal. GLES is a standard, there is compliance through
> > >
> > >*Shouts* ThunderX2 Workstation very loudly at this point, following by
> > >Linaro Developer Box, Macchiato bin.
> > >
> > >Linaro is more than Android!
> >
> > Quite. This is exactly the tension behind the dicussion - while arm64
> > machines are mainly mobile so far, we're finally starting to see
> > bigger and more capable systems that you'd actually be happy to use as
> > a desktop/laptop.
> >
> > Hence Wookey's question - is it possible to have a single sensible
> > answer for both the (large) mobile hardware user base and the
> > (smaller, but growing) bigger system users? We've seen conflicting
> > information in that thread, hence asking here! :-)
>
> That is Vulkan. There is no mobile&embedded vs desktop fracture
> like there is with GLES vs GL. In the design of the Vulkan standard
> that was one mistake they were trying to address.
>
> I personally would aim for Vulkan.

Plus there are several active efforts to do GL/GLES on top of Vulkan.

> > Is it true that most PCIe graphics cards (and drivers) will also
> > support GLES as well as GL? I've seen that asserted.
>
> As a litmus test, I don't see any GLES drivers on nVideas website.
> Both Vulkan and GL are there tho.

Intel supports GLES 1 and 2. And I think you can say any h/w Mesa
supports, supports both GL and GLES. At least to some version level.

Rob
___
linaro-dev mailing list
linaro-dev@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/linaro-dev

Re: qemu crash

2014-03-10 Thread Rob Herring
On Mon, Feb 24, 2014 at 9:30 PM, Daniel Lipsitt  wrote:
> I sent the below to the list before I was a subscriber and I don't think it
> made it through.
>
> -
>
> I don't know if I'm having a problem with QEMU or the linaro system image
> I'm using or something else, so I'm posting it here. Let me know if there's
> a more appropriate place.
>
> I'm experiencing the following crash:
>
> $ qemu-system-arm -machine beagle -nographic -sd
> linaro-saucy-nano-20140126.img
> qemu: fatal: Trying to execute code outside RAM or ROM at 0x402f0400
>
> R00=40014044 R01=402f0400 R02= R03=
> R04= R05= R06= R07=
> R08= R09= R10= R11=
> R12= R13=4020fcb0 R14= R15=402f0400
> PSR=41df -Z-- A sys32
> s00= s01= d00=
> s02= s03= d01=
> s04= s05= d02=
> s06= s07= d03=
> s08= s09= d04=
> s10= s11= d05=
> s12= s13= d06=
> s14= s15= d07=
> s16= s17= d08=
> s18= s19= d09=
> s20= s21= d10=
> s22= s23= d11=
> s24= s25= d12=
> s26= s27= d13=
> s28= s29= d14=
> s30= s31= d15=
> s32= s33= d16=
> s34= s35= d17=
> s36= s37= d18=
> s38= s39= d19=
> s40= s41= d20=
> s42= s43= d21=
> s44= s45= d22=
> s46= s47= d23=
> s48= s49= d24=
> s50= s51= d25=
> s52= s53= d26=
> s54= s55= d27=
> s56= s57= d28=
> s58= s59= d29=
> s60= s61= d30=
> s62= s63= d31=
> FPSCR: 
> Aborted (core dumped)
>
> Here's how I created the image:
>
> linaro-media-create --dev beagle \
> --hwpack hwpack_linaro-beaglebone_20140203-602_armhf_supported.tar.gz \
> --binary linaro-saucy-nano-20140126-627.tar.gz \
> --image-file linaro-saucy-nano-20140126.img
>
> I get the same error on the following OS/qemu combos:
>
> Ubuntu Trusty
>
> $ qemu-system-arm --version
> QEMU emulator version 1.7.0 (Debian 1.7.0+dfsg-3ubuntu1), Copyright (c)
> 2003-2008 Fabrice Bellard
>
> Ubuntu Saucy
>
> $ qemu --version
> QEMU emulator version 1.5.0 (Debian 1.5.0+dfsg-3ubuntu5.2), Copyright (c)
> 2003-2008 Fabrice Bellard
>
> MacOS 10.8.5
>
> QEMU emulator version 1.7.0 (qemu-linaro 2014.01), Copyright (c) 2003-2008
> Fabrice Bellard
>
> I get a similar error at a different address with this highbank image:
>
> http://releases.linaro.org/14.01/ubuntu/highbank/highbank-saucy_server_20140126-596.img.gz
>
> $ qemu-system-arm -machine highbank -nographic -sd
> highbank-saucy_server_20140126-596.img
> qemu: fatal: Trying to execute code outside RAM or ROM at 0x0800

Well, highbank does not support SD, but that's not your issue. Where's
your dtb and kernel?

I've recently sent some patches to qemu-devel to fix highbank. I think
this may be the smc call issue.

As for beagle, never used it so can't really help there.

Rob

___
linaro-dev mailing list
linaro-dev@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev


Re: 32bit binaries on 64-bit linux

2014-02-10 Thread Rob Herring
On Wed, Jan 29, 2014 at 12:34 PM, Arnd Bergmann  wrote:
> On Wednesday 29 January 2014 17:40:54 Wookey wrote:
>> +++ Arnd Bergmann [2014-01-29 18:14 +0100]:
>> > On Wednesday 29 January 2014 16:36:49 Wookey wrote:
>> > > Running 32-bit binaries is quite seriously broken until this is fixed. I
>> > > presume this currently isn't on anyone's list to fix? I'm not sure who's
>> > > list it should go on.
>> >
>> > Are you running with 4KB or 64KB page size in the kernel? IIRC you
>> > cannot really run 32-bit binaries with 64KB page size because of
>> > related issues.
>>
>> I'm not sure. The kernel config has:
>> CONFIG_HAVE_ARCH_TRANSPARENT_HUGEPAGE=y
>> # CONFIG_ARM64_64K_PAGES is not set
>> CONFIG_PAGEFLAGS_EXTENDED=y
>> # CONFIG_TRANSPARENT_HUGEPAGE is not set
>>
>> Is that using 64K pages or not?
>
> This is 4K pages, so I was on the wrong track here.
>
>> Is there a dynamic switch I can twiddle or does it need a kernel rebuild?
>
> If you turn on CONFIG_ARM64_64K_PAGES, you probably won't be able to
> load any 32-bit executables any more, or more will fail.

You definitely need 64K pages disabled and you also need CONFIG_COMPAT=y.

FWIW, I have 64-bit QEMU system level emulation working, but 32-bit
userspace is not working for me.

Rob

___
linaro-dev mailing list
linaro-dev@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev


Re: [PATCH] dt: add helper function to read u8 & u16 variables & arrays

2012-11-20 Thread Rob Herring
On 11/19/2012 10:45 PM, Viresh Kumar wrote:
> This adds following helper routines:
> - of_property_read_u8_array()
> - of_property_read_u16_array()
> - of_property_read_u8()
> - of_property_read_u16()
> 
> This expects arrays from DT to be passed as:
> - u8 array:
>   property = /bits/ 8 <0x50 0x60 0x70>;
> - u16 array:
>   property = /bits/ 16 <0x5000 0x6000 0x7000>;
> 
> Signed-off-by: Viresh Kumar 

Applied.

Rob

> ---
> V2->V3:
> - Expect u8 & u16 arrays to be passed using: /bits/ 8 or 16
> - remove common macro, as not much common now :(
> - Tested on ARM platform.
> 
>  drivers/of/base.c  | 77 
> ++
>  include/linux/of.h | 30 +
>  2 files changed, 107 insertions(+)
> 
> diff --git a/drivers/of/base.c b/drivers/of/base.c
> index af3b22a..f564e31 100644
> --- a/drivers/of/base.c
> +++ b/drivers/of/base.c
> @@ -671,12 +671,89 @@ struct device_node *of_find_node_by_phandle(phandle 
> handle)
>  EXPORT_SYMBOL(of_find_node_by_phandle);
>  
>  /**
> + * of_property_read_u8_array - Find and read an array of u8 from a property.
> + *
> + * @np:  device node from which the property value is to be read.
> + * @propname:name of the property to be searched.
> + * @out_value:   pointer to return value, modified only if return value 
> is 0.
> + * @sz:  number of array elements to read
> + *
> + * Search for a property in a device node and read 8-bit value(s) from
> + * it. Returns 0 on success, -EINVAL if the property does not exist,
> + * -ENODATA if property does not have a value, and -EOVERFLOW if the
> + * property data isn't large enough.
> + *
> + * dts entry of array should be like:
> + *   property = /bits/ 8 <0x50 0x60 0x70>;
> + *
> + * The out_value is modified only if a valid u8 value can be decoded.
> + */
> +int of_property_read_u8_array(const struct device_node *np,
> + const char *propname, u8 *out_values, size_t sz)
> +{
> + struct property *prop = of_find_property(np, propname, NULL);
> + const u8 *val;
> +
> + if (!prop)
> + return -EINVAL;
> + if (!prop->value)
> + return -ENODATA;
> + if ((sz * sizeof(*out_values)) > prop->length)
> + return -EOVERFLOW;
> +
> + val = prop->value;
> + while (sz--)
> + *out_values++ = *val++;
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(of_property_read_u8_array);
> +
> +/**
> + * of_property_read_u16_array - Find and read an array of u16 from a 
> property.
> + *
> + * @np:  device node from which the property value is to be read.
> + * @propname:name of the property to be searched.
> + * @out_value:   pointer to return value, modified only if return value 
> is 0.
> + * @sz:  number of array elements to read
> + *
> + * Search for a property in a device node and read 16-bit value(s) from
> + * it. Returns 0 on success, -EINVAL if the property does not exist,
> + * -ENODATA if property does not have a value, and -EOVERFLOW if the
> + * property data isn't large enough.
> + *
> + * dts entry of array should be like:
> + *   property = /bits/ 16 <0x5000 0x6000 0x7000>;
> + *
> + * The out_value is modified only if a valid u16 value can be decoded.
> + */
> +int of_property_read_u16_array(const struct device_node *np,
> + const char *propname, u16 *out_values, size_t sz)
> +{
> + struct property *prop = of_find_property(np, propname, NULL);
> + const __be16 *val;
> +
> + if (!prop)
> + return -EINVAL;
> + if (!prop->value)
> + return -ENODATA;
> + if ((sz * sizeof(*out_values)) > prop->length)
> + return -EOVERFLOW;
> +
> + val = prop->value;
> + while (sz--)
> + *out_values++ = be16_to_cpup(val++);
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(of_property_read_u16_array);
> +
> +/**
>   * of_property_read_u32_array - Find and read an array of 32 bit integers
>   * from a property.
>   *
>   * @np:  device node from which the property value is to be read.
>   * @propname:name of the property to be searched.
>   * @out_value:   pointer to return value, modified only if return value 
> is 0.
> + * @sz:  number of array elements to read
>   *
>   * Search for a property in a device node and read 32-bit value(s) from
>   * it. Returns 0 on success, -EINVAL if the property does not exist,
> diff --git a/include/linux/of.h b/include/linux/of.h
> index b4e50d5..bfdc130 100644
> --- a/include/linux/of.h
> +++ b/include/linux/of.h
> @@ -223,6 +223,10 @@ extern struct device_node *of_find_node_with_property(
>  extern struct property *of_find_property(const struct device_node *np,
>const char *name,
>int *lenp);
> +extern int of_property_read_u8_array(const struct device_node *np,
> + const char *propname

Re: [PATCH Resend V2] dt: add helper function to read u8 & u16 variables & arrays

2012-11-11 Thread Rob Herring
On 11/11/2012 11:27 AM, Viresh Kumar wrote:
> On 11 November 2012 19:42, Rob Herring  wrote:
>> On 11/06/2012 10:22 PM, viresh kumar wrote:
> 
>>> cluster0: cluster@0 {
>>> +   data1 = <0x50 0x60 0x70>;
>>> +   data2 = <0x5000 0x6000 0x7000>;
>>> +   data3 = <0x5000 0x6000 0x7000>;
>>
>> So there is a mismatch in our assumptions. You are just truncating
>> 32-bit values. I assumed you were using the 8 and 16 bit sizes that are
>> now supported in dts. I don't think we should just truncate values
>> blindly. We have support for specifying 8 and 16 values now so you
>> should use that and define that as part of a binding.
> 
> Sorry couldn't get your point at all :(
> What did you mean by "truncating 32 bit values" and how should we
> tell via DT, that the value passed is 8 bit, 16 bit or 32 bit?
> 

You are trying to retrieve an array of 8 or 16-bit values which are
stored as 32-bit values in dtb. Why not define them in the binding as 8
or 16 bit to begin with. Then there is never any ambiguity about their size.

I don't think the size is stored in the dtb. It is only in the dts. You
need to define the size in the binding definitions and use '/bits/'
annotation. With this the data is packed. Then the array function used
should match what the binding defines.

Rob


___
linaro-dev mailing list
linaro-dev@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev


Re: [PATCH Resend V2] dt: add helper function to read u8 & u16 variables & arrays

2012-11-11 Thread Rob Herring
On 11/06/2012 10:22 PM, viresh kumar wrote:
> On Tue, Nov 6, 2012 at 7:48 PM, Rob Herring  wrote:
>>> +#define of_property_read_array(_np, _pname, _out, _sz) 
>>>   \
> 
>>> + while (_sz--)   \
>>> + *_out++ = (typeof(*_out))be32_to_cpup(_val++);  \
> 
>> This will not work. You are incrementing _out by 1, 2, or 4 bytes, but
>> _val is always incremented by 4 bytes.
>>
>> According to the dtc commit adding this feature, the values are packed:
>>
>> With this patch the following property assignment:
>>
>> property = /bits/ 16 <0x1234 0x5678 0x0 0x>;
>>
>> is equivalent to:
>>
>> property = <0x12345678 0x>;
> 
> I thought of it a bit more and wasn't actually aligned with your explanation 
> :(
> 
> If that is the case, how will current implementation of u32 array will work
> if we pass something like: 0x88 0x8400 0x5890 from DT?
> 
> So, i did a dummy test of my current implementation, with following changes
> in one of my drivers:
> 
> dts changes:
> 
> cluster0: cluster@0 {
> +   data1 = <0x50 0x60 0x70>;
> +   data2 = <0x5000 0x6000 0x7000>;
> +   data3 = <0x5000 0x6000 0x7000>;

So there is a mismatch in our assumptions. You are just truncating
32-bit values. I assumed you were using the 8 and 16 bit sizes that are
now supported in dts. I don't think we should just truncate values
blindly. We have support for specifying 8 and 16 values now so you
should use that and define that as part of a binding.

Rob

>}
> 
> driver changes:
> 
> +void test(struct device_node *cluster)
> +{
> +   u8 data1[3];
> +   u16 data2[3];
> +   u32 data3[3], i;
> +
> +   of_property_read_u8_array(cluster, "data1", data1, 3);
> +   of_property_read_u16_array(cluster, "data2", data2, 3);
> +   of_property_read_u32_array(cluster, "data3", data3, 3);
> +
> +   for (i = 0; i < 3; i++) {
> +   printk(KERN_INFO "u8 %d: %x\n", i, data1[i]);
> +   printk(KERN_INFO "u16 %d: %x\n", i, data2[i]);
> +   printk(KERN_INFO "u32 %d: %x\n", i, data3[i]);
> +   }
> +}
> 
> And following is the output
> 
> [4.087205] u8 0: 50
> [4.093746] u16 0: 5000
> [4.101067] u32 0: 5000
> [4.109512] u8 1: 60
> [4.116036] u16 1: 6000
> [4.123357] u32 1: 6000
> [4.131718] u8 2: 70
> [4.138241] u16 2: 7000
> [4.145573] u32 2: 7000
> 
> which looks to be what we were looking for, isn't it?
> 
> Following is fixup for the doc comment missing:
> 
> commit 00803aed0781de451048df0d15a3e8c814a343c8
> Author: Viresh Kumar 
> Date:   Wed Nov 7 09:48:46 2012 +0530
> 
> fixup! dt: add helper function to read u8 & u16 variables & arrays
> ---
>  drivers/of/base.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/of/base.c b/drivers/of/base.c
> index fbb634b..4a6632e 100644
> --- a/drivers/of/base.c
> +++ b/drivers/of/base.c
> @@ -669,6 +669,7 @@ EXPORT_SYMBOL(of_find_node_by_phandle);
>   * @np:device node from which the property value is to be 
> read.
>   * @propname:  name of the property to be searched.
>   * @out_value: pointer to return value, modified only if return value is 0.
> + * @sz:number of array elements to read
>   *
>   * Search for a property in a device node and read 8-bit value(s) from
>   * it. Returns 0 on success, -EINVAL if the property does not exist,
> @@ -690,6 +691,7 @@ EXPORT_SYMBOL_GPL(of_property_read_u8_array);
>   * @np:device node from which the property value is to be 
> read.
>   * @propname:  name of the property to be searched.
>   * @out_value: pointer to return value, modified only if return value is 0.
> + * @sz:number of array elements to read
>   *
>   * Search for a property in a device node and read 16-bit value(s) from
>   * it. Returns 0 on success, -EINVAL if the property does not exist,
> @@ -712,6 +714,7 @@ EXPORT_SYMBOL_GPL(of_property_read_u16_array);
>   * @np:device node from which the property value is to be 
> read.
>   * @propname:  name of the property to be searched.
>   * @out_value: pointer to return value, modified only if return value is 0.
> + * @sz:number of array elements to read
>   *
>   * Search for a property in a device node and read 32-bit value(s) from
>   * it. Returns 0 on success, -EINVAL if the property does not exist,
> 


___
linaro-dev mailing list
linaro-dev@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev


Re: [PATCH Resend V2] dt: add helper function to read u8 & u16 variables & arrays

2012-11-06 Thread Rob Herring
On 10/25/2012 11:20 PM, Viresh Kumar wrote:
> This adds following helper routines:
> - of_property_read_u8_array()
> - of_property_read_u16_array()
> - of_property_read_u8()
> - of_property_read_u16()
> 
> First two actually share most of the code with of_property_read_u32_array(), 
> so
> the common part is taken out into a macro, which can be used by all three
> *_array() routines.
> 
> Signed-off-by: Viresh Kumar 
> ---
> V1->V2:
> -
> - Use typeof() in of_property_read_array() macro instead of passing type to it
> 
>  drivers/of/base.c  | 73 
> +++---
>  include/linux/of.h | 30 ++
>  2 files changed, 89 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/of/base.c b/drivers/of/base.c
> index af3b22a..039e178 100644
> --- a/drivers/of/base.c
> +++ b/drivers/of/base.c
> @@ -670,6 +670,64 @@ struct device_node *of_find_node_by_phandle(phandle 
> handle)
>  }
>  EXPORT_SYMBOL(of_find_node_by_phandle);
>  
> +#define of_property_read_array(_np, _pname, _out, _sz)   
> \
> + struct property *_prop = of_find_property(_np, _pname, NULL);   \
> + const __be32 *_val; \
> + \
> + if (!_prop) \
> + return -EINVAL; \
> + if (!_prop->value)  \
> + return -ENODATA;\
> + if ((_sz * sizeof(*_out)) > _prop->length)  \
> + return -EOVERFLOW;  \
> + \
> + _val = _prop->value;\
> + while (_sz--)   \
> + *_out++ = (typeof(*_out))be32_to_cpup(_val++);  \

This will not work. You are incrementing _out by 1, 2, or 4 bytes, but
_val is always incremented by 4 bytes.

According to the dtc commit adding this feature, the values are packed:

With this patch the following property assignment:

property = /bits/ 16 <0x1234 0x5678 0x0 0x>;

is equivalent to:

property = <0x12345678 0x>;


> + return 0;
> +
> +/**
> + * of_property_read_u8_array - Find and read an array of u8 from a property.
> + *
> + * @np:  device node from which the property value is to be read.
> + * @propname:name of the property to be searched.
> + * @out_value:   pointer to return value, modified only if return value 
> is 0.
> + *

Missing sz

> + * Search for a property in a device node and read 8-bit value(s) from
> + * it. Returns 0 on success, -EINVAL if the property does not exist,
> + * -ENODATA if property does not have a value, and -EOVERFLOW if the
> + * property data isn't large enough.
> + *
> + * The out_value is modified only if a valid u8 value can be decoded.
> + */
> +int of_property_read_u8_array(const struct device_node *np,
> + const char *propname, u8 *out_values, size_t sz)
> +{
> + of_property_read_array(np, propname, out_values, sz);
> +}
> +EXPORT_SYMBOL_GPL(of_property_read_u8_array);
> +
> +/**
> + * of_property_read_u16_array - Find and read an array of u16 from a 
> property.
> + *
> + * @np:  device node from which the property value is to be read.
> + * @propname:name of the property to be searched.
> + * @out_value:   pointer to return value, modified only if return value 
> is 0.
> + *

Ditto.

> + * Search for a property in a device node and read 16-bit value(s) from
> + * it. Returns 0 on success, -EINVAL if the property does not exist,
> + * -ENODATA if property does not have a value, and -EOVERFLOW if the
> + * property data isn't large enough.
> + *
> + * The out_value is modified only if a valid u16 value can be decoded.
> + */
> +int of_property_read_u16_array(const struct device_node *np,
> + const char *propname, u16 *out_values, size_t sz)
> +{
> + of_property_read_array(np, propname, out_values, sz);
> +}
> +EXPORT_SYMBOL_GPL(of_property_read_u16_array);
> +
>  /**
>   * of_property_read_u32_array - Find and read an array of 32 bit integers
>   * from a property.
> @@ -689,20 +747,7 @@ int of_property_read_u32_array(const struct device_node 
> *np,
>  const char *propname, u32 *out_values,
>  size_t sz)
>  {
> - struct property *prop = of_find_property(np, propname, NULL);
> - const __be32 *val;
> -
> - if (!prop)
> - return -EINVAL;
> - if (!prop->value)
> - return -ENODATA;
> - if ((sz * sizeof(*out_values)) > prop->length)
> - return -EOVERFLOW;
> -
> - val = prop->value;
> - while (sz--)
> - *o

Re: [PATCH v5 06/17] docs: Xen ARM DT bindings

2012-09-17 Thread Rob Herring
On 09/17/2012 12:38 PM, Stefano Stabellini wrote:
> Add a doc to describe the Xen ARM device tree bindings
> 
> 
> Changes in v5:
> 
> - add a comment about the size of the grant table memory region;
> - add a comment about the required presence of a GIC node;
> - specify that the described properties are part of a top-level
> "hypervisor" node;
> - specify #address-cells and #size-cells for the example.
> 
> 
> Changes in v4:
> 
> - "xen,xen" should be last as it is less specific;
> - update reg property using 2 address-cells and 2 size-cells.
> 
> 
> Signed-off-by: Stefano Stabellini 
> CC: devicetree-disc...@lists.ozlabs.org
> CC: David Vrabel 
> CC: Rob Herring 
> CC: Dave Martin 

This is so minimal now and it doesn't seem to have any overlap with kvm, so:

Acked-by: Rob Herring 

> ---
>  Documentation/devicetree/bindings/arm/xen.txt |   25 
> +
>  1 files changed, 25 insertions(+), 0 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/arm/xen.txt
> 
> diff --git a/Documentation/devicetree/bindings/arm/xen.txt 
> b/Documentation/devicetree/bindings/arm/xen.txt
> new file mode 100644
> index 000..0f7b9c2
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/arm/xen.txt
> @@ -0,0 +1,25 @@
> +* Xen hypervisor device tree bindings
> +
> +Xen ARM virtual platforms shall have a top-level "hypervisor" node with
> +the following properties:
> +
> +- compatible:
> + compatible = "xen,xen-", "xen,xen";
> +  where  is the version of the Xen ABI of the platform.
> +
> +- reg: specifies the base physical address and size of a region in
> +  memory where the grant table should be mapped to, using an
> +  HYPERVISOR_memory_op hypercall. The memory region is large enough to map
> +  the whole grant table (it is larger or equal to gnttab_max_grant_frames()).
> +
> +- interrupts: the interrupt used by Xen to inject event notifications.
> +  A GIC node is also required.
> +
> +
> +Example (assuming #address-cells = <2> and #size-cells = <2>):
> +
> +hypervisor {
> + compatible = "xen,xen-4.3", "xen,xen";
> + reg = <0 0xb000 0 0x2>;
> + interrupts = <1 15 0xf08>;
> +};
> 


___
linaro-dev mailing list
linaro-dev@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev


Re: [PATCH v4 06/24] docs: Xen ARM DT bindings

2012-09-17 Thread Rob Herring
On 09/14/2012 09:26 AM, Stefano Stabellini wrote:
> On Fri, 14 Sep 2012, Konrad Rzeszutek Wilk wrote:
>> On Fri, Sep 14, 2012 at 12:13:08PM +0100, Stefano Stabellini wrote:
>>> Add a doc to describe the Xen ARM device tree bindings
>>>
>>>
>>> Changes in v4:
>>>
>>> - "xen,xen" should be last as it is less specific;
>>> - update reg property using 2 address-cells and 2 size-cells.
>>>
>>>
>>> Signed-off-by: Stefano Stabellini 
>>> CC: devicetree-disc...@lists.ozlabs.org
>>> CC: David Vrabel 
>>> CC: Rob Herring 
>>> CC: Dave Martin 
>>> ---
>>>  Documentation/devicetree/bindings/arm/xen.txt |   22 ++
>>>  1 files changed, 22 insertions(+), 0 deletions(-)
>>>  create mode 100644 Documentation/devicetree/bindings/arm/xen.txt
>>>
>>> diff --git a/Documentation/devicetree/bindings/arm/xen.txt 
>>> b/Documentation/devicetree/bindings/arm/xen.txt
>>> new file mode 100644
>>> index 000..1f8f7d4
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/arm/xen.txt
>>> @@ -0,0 +1,22 @@
>>> +* Xen hypervisor device tree bindings
>>> +
>>> +Xen ARM virtual platforms shall have the following properties:
>>> +

State that they are part of top-level "hypervisor" node.

>>> +- compatible:
>>> +   compatible = "xen,xen-", "xen,xen";
>>> +  where  is the version of the Xen ABI of the platform.
>>> +
>>> +- reg: specifies the base physical address and size of a region in
>>> +  memory where the grant table should be mapped to, using an
>>> +  HYPERVISOR_memory_op hypercall. 
>>> +
>>> +- interrupts: the interrupt used by Xen to inject event notifications.
>>
>> Its singular here.. but in the example its plurar. What if you use
>> multiple of the same number ("16 0xf")?
> 
> The "interrupts" property in the example below is a standard property to
> describe interrupts. We just happen to declare only one interrupt.
> 
> From the device tree point of view it would be possible to declare more
> than one interrupt here, but Xen only supports one really.
> 
> Regarding the three cells used in the example (<1 15 0xf08>), they have
> a specific meaning in the GIC context:
> 
> """
>   The 1st cell is the interrupt type; 0 for SPI interrupts, 1 for PPI
>   interrupts.
> 
>   The 2nd cell contains the interrupt number for the interrupt type.
>   SPI interrupts are in the range [0-987].  PPI interrupts are in the
>   range [0-15].
> 
>   The 3rd cell is the flags, encoded as follows:
>   bits[3:0] trigger type and level flags.
>   1 = low-to-high edge triggered
>   2 = high-to-low edge triggered
>   4 = active high level-sensitive
>   8 = active low level-sensitive
>   bits[15:8] PPI interrupt cpu mask.  Each bit corresponds to each of
>   the 8 possible cpus attached to the GIC.  A bit set to '1' indicated
>   the interrupt is wired to that CPU.  Only valid for PPI interrupts.
> """
> 
> So <1 15 0xf08> means the last PPI.

Since it is a PPI, it is handled differently than a normal interrupt.
That is fine, but you should somehow state that a GIC node is also required.

> 
>>> +
>>> +
>>> +Example:
>>> +
>>> +hypervisor {
>>> +   compatible = "xen,xen-4.3", "xen,xen";
>>> +   reg = <0 0xb000 0 0x2>;
>>
>> So two grant tables?
>>
>> Hm, physical address is zero, and the size is 0xbignumber?
>> Or is the '0' denotating a seperator of arguments, so it is
>> 0xb000.. for physical address and 0x2 for size?
> 
> from http://devicetree.org/Device_Tree_Usage:
> 
> "Each addressable device gets a reg which is a list of tuples in the
> form reg =  Each tuple represents an address range used by the device. Each address
> value is a list of one or more 32 bit integers called cells. Similarly,
> the length value can either be a list of cells, or empty."
> 
> In this case the address is: [0 0xb000], that means
> 0xb000, and the length is [0 0x2], that means
> 0x0002.

But the size depends on #size-cells and #address-cells. I would expect
those to be 1 for a 32-bit guest.

Rob


___
linaro-dev mailing list
linaro-dev@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev


Re: [PATCH v3 06/25] docs: Xen ARM DT bindings

2012-09-12 Thread Rob Herring
On 09/12/2012 01:14 PM, Stefano Stabellini wrote:
> On Wed, 12 Sep 2012, Stefano Stabellini wrote:
>> On Tue, 28 Aug 2012, Rob Herring wrote:
>>> You should look at ePAPR 1.1 which defines hypervisor related bindings.
>>> While it is a PPC doc, we should reuse or extend what makes sense.
>>>
>>> See section 7.5:
>>>
>>> https://www.power.org/resources/downloads/Power_ePAPR_APPROVED_v1.1.pdf
>>
>> Thanks for the link, I wasn't aware of ePAPR.
>>
>> The hypervisor node defined by ePAPR is not very different from what I
>> am proposing. Should I try to be compatible with the hypervisor
>> specification above (as in compatible = "epapr,hypervisor-1.1")?
>> Or should I just use it as a reference for my own specification?
>>
>> Personally I would rather avoid full compatibility with ePAPR.
> 
> In particular reading section 7.5, these are the required properties of
> the ePAPR hypervisor node:
> 
> - compatible = "epapr,hypervisor-1.1"
> compared to what I am proposing, it is laking information about what
> hypervisor we are talking about (xen, kvm, vmware, etc) and the version
> of the ABI (xen-4.2).

compatible properties are often changed. If we do deviate on the rest of
the binding, then it needs to be different.

Turns out that powerpc KVM guests use "linux,kvm" and "epapr,hypervisor"
doesn't even appear in the kernel.

We also perhaps have to consider the possibility of Xen on PowerPC. Then
alignment is more important.

> - hcall-instructions
> potentially interesting, but given that for Xen we are quite happy with
> HVC, we are not going to add any secondary hypercall mechanisms,
> therefore at the moment it would just result in a BUG if the specified
> hcall instruction is != HVC. Besides if somebody else wanted to
> implemented the Xen hypercall interface in a different way they could
> just reimplement the hypercall wrappers, that would be easier than
> trying to do it with this property.

It's really about the parameters passed with the HVC. The ePAPR may be a
good model for defining this. Just grepping "hypervisor" under
arch/powerpc, it's pretty clear hypervisor support happened first and
the ePAPR came second. Hopefully we can avoid that for ARM.

> - guest-id
> we usually make a point in trying not to tell the guest its own domid
> because if the VM gets migrated to a different host it acquires a new
> domid, therefore it should not rely on it.
> 
> - guest-name
> we could pass the guest name here, but I don't see how it could be
> of any use.
> 

I have no issue with these being optional.

> 
> On the other hand, thinking more about what Xen needs in the device
> tree, I think we could improve the current spec by clarifying the
> meaning of the memory region and interrupt properties we currently
> require. I thought about moving them to two separate children node with
> an explicit name:
> 
> ---
> 
> * Xen hypervisor device tree bindings
> 

I really think we need to define ARM hypervisor device tree bindings
first, then Xen specific bindings as an addition to that. I worry that
the KVM folks aren't paying attention and then want something different
later on.

> Xen ARM virtual platforms shall have the following properties and
> children nodes:
> 
> - compatible property:
>   compatible = "xen,xen", "xen,xen-";

"xen,xen" should be last as it is less specific.

>   where  is the version of the Xen ABI of the platform.
> 
> - grant_table child with the following properties:
> - name:
>   name = "grant_table";

What's a grant table?

>   - reg: specifies the base physical address and size of a region in
>   memory where the grant table should be mapped to, using an
> HYPERVISOR_memory_op hypercall. 
> 
> - events child with the following properties:
> - name:
> name = "events";
>   - interrupts: the interrupt used by Xen to inject event notifications.

Why a child node? Just an interrupts property alone should be fine. If
you have cases with different number of interrupts, the compatible
property can distinguish that.

Rob

> 
> 
> Example:
> hypervisor {
>   compatible = "xen,xen", "xen,xen-4.2";
>   #address-cells = <1>;
>   #size-cells = <1>;
>   #interrupt-cells = <3>;
>   ranges = <0xb000 0xb000 0x2>;
> 
>   grant_table {
>   name = "grant_table";
>   reg = <0xb000 0x2>;
>   };
> 
>   events {
>   name = "events";
>   interrupts = <1 15 0xf08>;
>   };
>   };
> 


___
linaro-dev mailing list
linaro-dev@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev


Re: [PATCH v3 06/25] docs: Xen ARM DT bindings

2012-08-27 Thread Rob Herring
On 08/16/2012 10:35 AM, Stefano Stabellini wrote:
> Add a doc to describe the Xen ARM device tree bindings
> 
> Signed-off-by: Stefano Stabellini 
> CC: devicetree-disc...@lists.ozlabs.org
> CC: David Vrabel 
> ---
>  Documentation/devicetree/bindings/arm/xen.txt |   22 ++
>  1 files changed, 22 insertions(+), 0 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/arm/xen.txt
> 
> diff --git a/Documentation/devicetree/bindings/arm/xen.txt 
> b/Documentation/devicetree/bindings/arm/xen.txt
> new file mode 100644
> index 000..ec6d884
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/arm/xen.txt
> @@ -0,0 +1,22 @@
> +* Xen hypervisor device tree bindings
> +
> +Xen ARM virtual platforms shall have the following properties:
> +
> +- compatible:
> + compatible = "xen,xen", "xen,xen-";
> +  where  is the version of the Xen ABI of the platform.
> +
> +- reg: specifies the base physical address and size of a region in
> +  memory where the grant table should be mapped to, using an
> +  HYPERVISOR_memory_op hypercall. 
> +
> +- interrupts: the interrupt used by Xen to inject event notifications.

You should look at ePAPR 1.1 which defines hypervisor related bindings.
While it is a PPC doc, we should reuse or extend what makes sense.

See section 7.5:

https://www.power.org/resources/downloads/Power_ePAPR_APPROVED_v1.1.pdf

Rob

> +
> +
> +Example:
> +
> +hypervisor {
> + compatible = "xen,xen", "xen,xen-4.3";
> + reg = <0xb000 0x2>;
> + interrupts = <1 15 0xf08>;
> +};
> 


___
linaro-dev mailing list
linaro-dev@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev


Re: [PATCH 01/24] arm: initial Xen support

2012-08-01 Thread Rob Herring
On 07/26/2012 10:33 AM, Stefano Stabellini wrote:
> - Basic hypervisor.h and interface.h definitions.
> - Skelethon enlighten.c, set xen_start_info to an empty struct.
> - Do not limit xen_initial_domain to PV guests.
> 
> The new code only compiles when CONFIG_XEN is set, that is going to be
> added to arch/arm/Kconfig in a later patch.
> 
> Signed-off-by: Stefano Stabellini 
> ---
>  arch/arm/Makefile |1 +
>  arch/arm/include/asm/hypervisor.h |6 +++
>  arch/arm/include/asm/xen/hypervisor.h |   19 ++
>  arch/arm/include/asm/xen/interface.h  |   64 
> +

These headers don't seem particularly ARM specific. Could they be moved
to asm-generic or include/linux?

Rob

>  arch/arm/xen/Makefile |1 +
>  arch/arm/xen/enlighten.c  |   35 ++
>  include/xen/xen.h |2 +-
>  7 files changed, 127 insertions(+), 1 deletions(-)
>  create mode 100644 arch/arm/include/asm/hypervisor.h
>  create mode 100644 arch/arm/include/asm/xen/hypervisor.h
>  create mode 100644 arch/arm/include/asm/xen/interface.h
>  create mode 100644 arch/arm/xen/Makefile
>  create mode 100644 arch/arm/xen/enlighten.c
> 
> diff --git a/arch/arm/Makefile b/arch/arm/Makefile
> index 0298b00..70aaa82 100644
> --- a/arch/arm/Makefile
> +++ b/arch/arm/Makefile
> @@ -246,6 +246,7 @@ endif
>  core-$(CONFIG_FPE_NWFPE) += arch/arm/nwfpe/
>  core-$(CONFIG_FPE_FASTFPE)   += $(FASTFPE_OBJ)
>  core-$(CONFIG_VFP)   += arch/arm/vfp/
> +core-$(CONFIG_XEN)   += arch/arm/xen/
>  
>  # If we have a machine-specific directory, then include it in the build.
>  core-y   += arch/arm/kernel/ arch/arm/mm/ 
> arch/arm/common/
> diff --git a/arch/arm/include/asm/hypervisor.h 
> b/arch/arm/include/asm/hypervisor.h
> new file mode 100644
> index 000..b90d9e5
> --- /dev/null
> +++ b/arch/arm/include/asm/hypervisor.h
> @@ -0,0 +1,6 @@
> +#ifndef _ASM_ARM_HYPERVISOR_H
> +#define _ASM_ARM_HYPERVISOR_H
> +
> +#include 
> +
> +#endif
> diff --git a/arch/arm/include/asm/xen/hypervisor.h 
> b/arch/arm/include/asm/xen/hypervisor.h
> new file mode 100644
> index 000..d7ab99a
> --- /dev/null
> +++ b/arch/arm/include/asm/xen/hypervisor.h
> @@ -0,0 +1,19 @@
> +#ifndef _ASM_ARM_XEN_HYPERVISOR_H
> +#define _ASM_ARM_XEN_HYPERVISOR_H
> +
> +extern struct shared_info *HYPERVISOR_shared_info;
> +extern struct start_info *xen_start_info;
> +
> +/* Lazy mode for batching updates / context switch */
> +enum paravirt_lazy_mode {
> + PARAVIRT_LAZY_NONE,
> + PARAVIRT_LAZY_MMU,
> + PARAVIRT_LAZY_CPU,
> +};
> +
> +static inline enum paravirt_lazy_mode paravirt_get_lazy_mode(void)
> +{
> + return PARAVIRT_LAZY_NONE;
> +}
> +
> +#endif /* _ASM_ARM_XEN_HYPERVISOR_H */
> diff --git a/arch/arm/include/asm/xen/interface.h 
> b/arch/arm/include/asm/xen/interface.h
> new file mode 100644
> index 000..6c3ab59
> --- /dev/null
> +++ b/arch/arm/include/asm/xen/interface.h
> @@ -0,0 +1,64 @@
> +/**
> + * Guest OS interface to ARM Xen.
> + *
> + * Stefano Stabellini , Citrix, 2011
> + */
> +
> +#ifndef _ASM_ARM_XEN_INTERFACE_H
> +#define _ASM_ARM_XEN_INTERFACE_H
> +
> +#include 
> +
> +#define __DEFINE_GUEST_HANDLE(name, type) \
> + typedef type * __guest_handle_ ## name
> +
> +#define DEFINE_GUEST_HANDLE_STRUCT(name) \
> + __DEFINE_GUEST_HANDLE(name, struct name)
> +#define DEFINE_GUEST_HANDLE(name) __DEFINE_GUEST_HANDLE(name, name)
> +#define GUEST_HANDLE(name)__guest_handle_ ## name
> +
> +#define set_xen_guest_handle(hnd, val)   \
> + do {\
> + if (sizeof(hnd) == 8)   \
> + *(uint64_t *)&(hnd) = 0;\
> + (hnd) = val;\
> + } while (0)
> +
> +#ifndef __ASSEMBLY__
> +/* Guest handles for primitive C types. */
> +__DEFINE_GUEST_HANDLE(uchar, unsigned char);
> +__DEFINE_GUEST_HANDLE(uint,  unsigned int);
> +__DEFINE_GUEST_HANDLE(ulong, unsigned long);
> +DEFINE_GUEST_HANDLE(char);
> +DEFINE_GUEST_HANDLE(int);
> +DEFINE_GUEST_HANDLE(long);
> +DEFINE_GUEST_HANDLE(void);
> +DEFINE_GUEST_HANDLE(uint64_t);
> +DEFINE_GUEST_HANDLE(uint32_t);
> +
> +/* Maximum number of virtual CPUs in multi-processor guests. */
> +#define MAX_VIRT_CPUS 1
> +
> +struct arch_vcpu_info { };
> +struct arch_shared_info { };
> +
> +/* XXX: Move pvclock definitions some place arch independent */
> +struct pvclock_vcpu_time_info {
> + u32   version;
> + u32   pad0;
> + u64   tsc_timestamp;
> + u64   system_time;
> + u32   tsc_to_system_mul;
> + s8tsc_shift;
> + u8flags;
> + u8pad[2];
> +} __attribute__((__packed__)); /* 32 bytes */
> +
> +struct pvclock_wall_clock {
> + u32   version;
> + u32   sec;
> + u32   nsec;
> +} __attribu

Re: [PATCH] arm: Handle device tree memory regions larger than 4GB

2012-07-07 Thread Rob Herring
On 07/07/2012 05:36 AM, Peter Maydell wrote:
> On 6 July 2012 20:26, Dave Martin  wrote:
>> On Fri, Jul 06, 2012 at 07:07:35PM +0100, Peter Maydell wrote:
>>> On 6 July 2012 00:27, Rob Herring  wrote:
>>>> I would just change arm_add_memory to use phys_addr_t for the size
>>>> param. This ultimately calls memblock functions which use phys_addr_t
>>>> for sizes.
>>>
>>> So I have a patch that does this which basically works. However
>>> there is a bit I'm not sure about. arm_add_memory() does this:
>>>bank->size = size & PAGE_MASK;
>>>
>>> in an attempt to clear the bottom bits of the size. However,
>>> since PAGE_MASK is defined as:
>>>  #define PAGE_SIZE   (_AC(1,UL) << PAGE_SHIFT)
>>>  #define PAGE_MASK   (~(PAGE_SIZE-1))
>>>
>>> PAGE_MASK is a 32 bit unsigned constant and so this & will
>>> clear the top 32 bits of bank->size.
>>>
>>> I'm really not sure what the best way to fix this is; suggestions?
>>
>> Maybe something like
>>
>> ~(phys_addr_t)~PAGE_MASK
>>
>> or
>>
>> ~(phys_addr_t)(PAGE_SIZE - 1)
>>
>> would work.
> 
> Mmm. It feels a bit unsatisfactory that an "obviously correct"
> line of code like "size &= ~PAGE_MASK" could actually be subtly
> wrong (seems like the kind of thing that could easily slip
> through code review), so I was wondering if maybe redefining
> PAGE_MASK so it didn't do the wrong thing on 64 bit types
> would be better. (That's definitely way out of my depth though.)
> 

While there is a mixture of usage of PAGE_MASK in the kernel, I think
correct usage is with virt addresses. For phys addresses, there is
PHYS_MASK which is sized correctly for LPAE.

You really should post this on the lakml list.

Rob

> -- PMM
> 



___
linaro-dev mailing list
linaro-dev@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev


Re: [PATCH] arm: Handle device tree memory regions larger than 4GB

2012-07-05 Thread Rob Herring
On 07/05/2012 11:48 AM, Peter Maydell wrote:
> Device tree memory regions may have sizes larger than 4GB.
> Instead of silently truncating a 64 bit size when we pass it
> to arm_add_memory(), split large regions into 2GB chunks.
> 
> Signed-off-by: Peter Maydell 
> ---
> With this patch, I can take a device tree which has been tweaked
> so its #address-cells and #size-cells are both 2, and boot it on
> a QEMU vexpress-a15 model with >4GB of RAM, and have the kernel
> actually detect the right amount of RAM. [the qemu bit needs some
> patches I haven't posted yet.]
> 
> Since I'm not really a kernel dev I thought I'd post this to
> linaro-dev first in the hope of a bit of friendly local review
> before venturing onto lkml :-)
> (Apologies to those on cc who got this twice because I mistyped
> the linaro-dev email address.)

We'll still find you. ;)

>  arch/arm/kernel/devtree.c |5 +
>  1 file changed, 5 insertions(+)
> 
> diff --git a/arch/arm/kernel/devtree.c b/arch/arm/kernel/devtree.c
> index bee7f9d..79a6e66 100644
> --- a/arch/arm/kernel/devtree.c
> +++ b/arch/arm/kernel/devtree.c
> @@ -26,6 +26,11 @@
>  
>  void __init early_init_dt_add_memory_arch(u64 base, u64 size)
>  {
> + while (size > 0x8000) {
> + arm_add_memory(base, 0x8000);
> + base += 0x8000;
> + size -= 0x8000;
> + }
>   arm_add_memory(base, size);

I would just change arm_add_memory to use phys_addr_t for the size
param. This ultimately calls memblock functions which use phys_addr_t
for sizes.

One thing I noticed is ATAGs will be broken for LPAE. That's probably
fine because if you can fix your bootloader for new ATAGs, then you can
support DT.

Rob

>  }
>  
> 



___
linaro-dev mailing list
linaro-dev@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev


Re: Kernel not booting with Linaro GCC?

2012-06-13 Thread Rob Herring
On 06/13/2012 03:26 PM, Michael Hope wrote:
> On 14 June 2012 04:22, Rob Herring  wrote:
>> On 06/10/2012 05:31 PM, Michael Hope wrote:
>>> There's an interaction between Linaro GCC or FSF GCC 4.7 and Linux
>>> kernels before 3.2 which causes the kernel to halt straight after
>>> showing 'Uncompressing Linux'.  The question comes up every couple of
>>> months so I've blogged about it:
>>>  http://seabright.co.nz/2012/06/11/kernel-not-booting-with-linaro-gcc/
>>>
>>> """
>>> Is your ARM Linux kernel not booting when building with Linaro GCC or
>>> FSF GCC 4.7? Does it halt shortly after showing ‘Uncompressing Linux’?
>>> You may have run into an interaction between older kernels and the new
>>> unaligned access support in GCC. This affects Linaro GCC from
>>> 4.6-2011.11 onwards, GCC from 4.7.0 on, and kernels earlier than 3.2
>>> including the Galaxy Nexus Icecream Sandwich release.
>>>
>>> The work-around is to add -mno-unaligned-access to KBUILD_CFLAGS in
>>> the top level kernel Makefile or to backport
>>> 8428e84d42179c2a00f5f6450866e70d802d1d05 from the current kernel tree.
>>>
>>> ARMv6K and later processors have hardware support for doing unaligned
>>> loads and stores which is faster than the old byte-by-byte/recombine
>>> that was done in software. Later versions of GCC use this to do
>>> quicker loads when working on known unaligned data, such as when
>>> working on a protocol buffer or a packed structure.
>>>
>>> The CPU can be configured to trap on unaligned access. This trap is
>>> off at reset, but pre 3.2 kernels turn this on during the initial
>>> boot. An interaction between -fconserve-stack and -munaligned-access
>>> on a char buffer lead to an unaligned access, which causes a trap,
>>> which causes the kernel to halt.
>>>
>>> This does not affect userspace programs as they run with the trap turned 
>>> off.
>>> """
>>
>> I've also hit this with u-boot if I enable armv7-a builds. Mainline
>> u-boot generally builds using -march=armv5 and unaligned accesses
>> disabled in h/w. Generally u-boot starts but dies on certain commands. I
>> think there may be other u-boot issues with v7 compiles on newer gcc
>> versions, but haven't debugged things further.
> 
> Note that this is done through a unaligned access trap that is off by
> default.  A quick grep through u-boot git shows that it defines CR_A
> but doesn't use it.
> 

The A bit is on in u-boot.

armv7 start.S:

mrc p15, 0, r0, c1, c0, 0
bic r0, r0, #0x2000 @ clear bits 13 (--V-)
bic r0, r0, #0x0007 @ clear bits 2:0 (-CAM)
orr r0, r0, #0x0002 @ set bit 1 (--A-) Align
orr r0, r0, #0x0800 @ set bit 11 (Z---) BTB
#ifdef CONFIG_SYS_ICACHE_OFF
bic r0, r0, #0x1000 @ clear bit 12 (I) I-cache
#else
orr r0, r0, #0x1000 @ set bit 12 (I) I-cache
#endif
mcr p15, 0, r0, c1, c0, 0
mov pc, lr  @ back to my caller


There is no trap handler in u-boot. I've turned off the A bit, but still
had some problems which I did not debug further.

Rob

> -- Michael


___
linaro-dev mailing list
linaro-dev@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev


Re: Kernel not booting with Linaro GCC?

2012-06-13 Thread Rob Herring
On 06/10/2012 05:31 PM, Michael Hope wrote:
> There's an interaction between Linaro GCC or FSF GCC 4.7 and Linux
> kernels before 3.2 which causes the kernel to halt straight after
> showing 'Uncompressing Linux'.  The question comes up every couple of
> months so I've blogged about it:
>  http://seabright.co.nz/2012/06/11/kernel-not-booting-with-linaro-gcc/
> 
> """
> Is your ARM Linux kernel not booting when building with Linaro GCC or
> FSF GCC 4.7? Does it halt shortly after showing ‘Uncompressing Linux’?
> You may have run into an interaction between older kernels and the new
> unaligned access support in GCC. This affects Linaro GCC from
> 4.6-2011.11 onwards, GCC from 4.7.0 on, and kernels earlier than 3.2
> including the Galaxy Nexus Icecream Sandwich release.
> 
> The work-around is to add -mno-unaligned-access to KBUILD_CFLAGS in
> the top level kernel Makefile or to backport
> 8428e84d42179c2a00f5f6450866e70d802d1d05 from the current kernel tree.
> 
> ARMv6K and later processors have hardware support for doing unaligned
> loads and stores which is faster than the old byte-by-byte/recombine
> that was done in software. Later versions of GCC use this to do
> quicker loads when working on known unaligned data, such as when
> working on a protocol buffer or a packed structure.
> 
> The CPU can be configured to trap on unaligned access. This trap is
> off at reset, but pre 3.2 kernels turn this on during the initial
> boot. An interaction between -fconserve-stack and -munaligned-access
> on a char buffer lead to an unaligned access, which causes a trap,
> which causes the kernel to halt.
> 
> This does not affect userspace programs as they run with the trap turned off.
> """

I've also hit this with u-boot if I enable armv7-a builds. Mainline
u-boot generally builds using -march=armv5 and unaligned accesses
disabled in h/w. Generally u-boot starts but dies on certain commands. I
think there may be other u-boot issues with v7 compiles on newer gcc
versions, but haven't debugged things further.

Rob

> 
> -- Michael
> 
> ___
> linaro-dev mailing list
> linaro-dev@lists.linaro.org
> http://lists.linaro.org/mailman/listinfo/linaro-dev


___
linaro-dev mailing list
linaro-dev@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev


Re: Making ARM multiplatform kernels DT-only?

2012-05-04 Thread Rob Herring
On 05/04/2012 07:20 AM, Arnd Bergmann wrote:
> On Thursday 03 May 2012, Russell King - ARM Linux wrote:
>> I'm basing my comments off mach-zynq.
> 
> It's a different question because mach-zynq is already DT-only, but we
> can also discuss this for a bit.
> 
>> How about we take the following steps towards it?
>>
>> 1. create arch/arm/include/mach/ which contains standardized headers
>>for DT based implementations.  This must include all headers included
>>by asm/ or linux/ includes.  This will also be the only mach/ header
>>directory included for code outside of arch/arm/mach-*.  This also
>>acts as the 'default' set of mach/* includes for stuff like timex.h
>>and the empty hardware.h
>>
>> 2. DT based mach-* directories do not have an include directory; their
>>include files must be located in the main include/ heirarchy if shared
>>with other parts of the kernel, otherwise they must be in the mach-*
>>directory.
> 
> My idea for the header files was slightly different, reorganizing only
> the headers that actually conflict between the platforms renaming the
> ones that conflict by name but not by content.
> 
> I know you are aware of my experiment with just renaming the header files
> from mach/*.h to mach-*/*.h, and that has helped me a lot in understanding
> the specific problems. I don't care about the specific names of the headers
> but it has helped so far in identifying which drivers are already relying
> on a specific platform's version of a header and which ones multiplex
> between different platforms (e.g. sa1100/pxa/mmp or s3c*/s5p*/exynos)
> and need more work. 
> 
> I also wouldn't change anything for the current configurations where
> you only have one mach-* directory at a time (or the samsung speciality).
> 
> My plan is to have multiplatform kernels in parallel with what we have now,
> so we can avoid breaking working machines but also play with multiplatform
> configurations at the same time for a subset of the platforms and with
> certain restrictions (not all board files, not all drivers, no generic
> early printk, ...).
> 

Many of the headers are simply platform_data structs which may still be
needed on DT platforms, but could be moved elsewhere.

>> 3. Allow build multiple mach-* directories (which we already do... see
>>the samsung stuff.)
> 
> Incidentally, the samsung headers are what are currently causing the most
> headaches regarding the header files, because they use a lot of files
> with soc-specific definitions for the same symbols, which means significant
> reorganization of the code using to to turn that into run-time selectable
> values.
> 
>> We still have irqs.h being SoC dependent, and we still haven't taken
>> debug-macros.S far enough along to get rid of that.
> 
> I believe the irqs.h conflict is only for the NR_IRQS constant, all other
> defines in there should only be used inside of the mach-* directory,
> or not at all for fully DT-based platforms.

A DT-enabled platform does not need irqs.h or NR_IRQS. SPARSE_IRQ should
be selected for DT. However, some DT enabled platforms don't have all
irq chips converted to domains and may still need to set the mach .nr_irqs.

> 
>> Then there's also the problem of uncompress.h.  The last piece of the
>> puzzle is the common clock stuff.

The smp/hotplug/localtimer related functions are still global. Marc Z
has posted patches for this, but I haven't seen recent activity. This
and clocks were the 2 main issues I saw trying to build 2 platforms
together. highbank and picoxcell could be built together since only
highbank has clocks and smp.

gpio.h is still required, but empty for most platforms.

Rob

> Initially, I think we can live without debug-macros.S and uncompress.h
> and change the code using those to just not output anything when
> MULTIPLATFORM is enabled: you'd have to disable MULTIPLATFORM in order
> to debug the early boot process and hope that any bugs are not
> specific to multiplatform configurations. In the long run, we probably
> want to have a better solution, but it's not on my hotlist.
> 
> The common clock support OTOH is definitely a requirement as soon as
> we want to actually run multiplatform kernels rather than just building
> them.
>  
>> So, I think we're still a way off it yet - maybe six months or so.
> 
> True, but in order to work on the points you raised and a few others,
> I would like to know where we're heading because it does impact
> some decisions like whether we need to make all initcalls in non-DT
> board files aware of potentially being run on other platforms.
> 
>   Arnd
> 
> ___
> linux-arm-kernel mailing list
> linux-arm-ker...@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel


___
linaro-dev mailing list
linaro-dev@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev


Re: [PATCH v7 1/3] Documentation: common clk API

2012-03-16 Thread Rob Herring
On 03/16/2012 06:47 PM, Sascha Hauer wrote:
> Hi Paul,
> 
> On Fri, Mar 16, 2012 at 04:21:17PM -0600, Paul Walmsley wrote:
>> Hi
>>
>> On Fri, 16 Mar 2012, Arnd Bergmann wrote:
>>
>>
>> If the common clock code is to go upstream now, it should be marked as 
>> experimental.
> 
> No, please don't do this. This effectively marks the architectures using
> the generic clock framework experimental. We can mark drivers as 'you
> have been warned', but marking an architecture as experimental is the
> wrong sign for both its users and people willing to adopt the framework.
> Also we get this:
> 
> warning: (ARCH_MX1 && MACH_MX21 && ARCH_MX25 && MACH_MX27) selects COMMON_CLK 
> which has unmet direct dependencies (EXPERIMENTAL)
> 
> (and no, I don't want to support to clock frameworks in parallel)
> 

+1

For simple users at least, the api is perfectly adequate and it is not
experimental (unless new means experimental).

Rob

>> This is because we know the API is not well-defined, and 
>> that both the API and the underlying mechanics will almost certainly need 
>> to change for non-trivial uses of the rate changing code (e.g., DVFS with 
>> external I/O devices).
> 
> Please leave DVFS out of the game. DVFS will use the clock framework for
> the F part and the regulator framework for the V part, but the clock
> framework should not get extended with DVFS features. The notifiers we
> currently have in the clock framework should give enough information
> for DVFS implementations. Even if they don't and we have to change
> something here this will have no influence on the architectures
> implementing their clock tree with the common clock framework.
> 
> Sascha
> 


___
linaro-dev mailing list
linaro-dev@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev


Re: [PATCH v6 3/3] clk: basic clock hardware types

2012-03-14 Thread Rob Herring
On 03/14/2012 01:23 PM, Sascha Hauer wrote:
> On Fri, Mar 09, 2012 at 11:54:24PM -0800, Mike Turquette wrote:
>> Many platforms support simple gateable clocks, fixed-rate clocks,
>> adjustable divider clocks and multi-parent multiplexer clocks.
>>
>> This patch introduces basic clock types for the above-mentioned hardware
>> which share some common characteristics.
>>
>> Based on original work by Jeremy Kerr and contribution by Jamie Iles.
>> Dividers and multiplexor clocks originally contributed by Richard Zhao &
>> Sascha Hauer.
>>
>> Signed-off-by: Mike Turquette 
>> Signed-off-by: Mike Turquette 
>> Cc: Russell King 
>> Cc: Jeremy Kerr 
>> Cc: Thomas Gleixner 
>> Cc: Arnd Bergman 
>> Cc: Paul Walmsley 
>> Cc: Shawn Guo 
>> Cc: Sascha Hauer 
>> Cc: Jamie Iles 
>> Cc: Richard Zhao 
>> Cc: Saravana Kannan 
>> Cc: Magnus Damm 
>> Cc: Rob Herring 
>> Cc: Mark Brown 
>> Cc: Linus Walleij 
>> Cc: Stephen Boyd 
>> Cc: Amit Kucheria 
>> Cc: Deepak Saxena 
>> Cc: Grant Likely 
>> Cc: Andrew Lunn 
>> ---
>> Changes since v5:
>>  * standardized the hw-specific locking in the basic clock types
>>  * export the individual ops for each basic clock type
>>  * improve registration for single-parent basic clocks (thanks Sascha)
>>  * fixed bugs in gate clock's static initializers (thanks Andrew)
>>
>>  drivers/clk/Makefile |3 +-
>>  drivers/clk/clk-divider.c|  204 
>> ++
>>  drivers/clk/clk-fixed-rate.c |   82 +
>>  drivers/clk/clk-gate.c   |  157 
>>  drivers/clk/clk-mux.c|  123 +
>>  include/linux/clk-private.h  |  124 +
>>  include/linux/clk-provider.h |  127 ++
>>  7 files changed, 819 insertions(+), 1 deletions(-)
>>  create mode 100644 drivers/clk/clk-divider.c
>>  create mode 100644 drivers/clk/clk-fixed-rate.c
>>  create mode 100644 drivers/clk/clk-gate.c
>>  create mode 100644 drivers/clk/clk-mux.c
>>
>> diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile
>> index ff362c4..1f736bc 100644
>> --- a/drivers/clk/Makefile
>> +++ b/drivers/clk/Makefile
>> @@ -1,3 +1,4 @@
>>  
>>  obj-$(CONFIG_CLKDEV_LOOKUP) += clkdev.o
>> -obj-$(CONFIG_COMMON_CLK)+= clk.o
>> +obj-$(CONFIG_COMMON_CLK)+= clk.o clk-fixed-rate.o clk-gate.o \
>> +   clk-mux.o clk-divider.o
>> diff --git a/drivers/clk/clk-divider.c b/drivers/clk/clk-divider.c
>> new file mode 100644
>> index 000..c0c4e0b
>> --- /dev/null
>> +++ b/drivers/clk/clk-divider.c
>> @@ -0,0 +1,204 @@
>> +/*
>> + * Copyright (C) 2011 Sascha Hauer, Pengutronix 
>> + * Copyright (C) 2011 Richard Zhao, Linaro 
>> + * Copyright (C) 2011-2012 Mike Turquette, Linaro Ltd 
>> 
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License version 2 as
>> + * published by the Free Software Foundation.
>> + *
>> + * Adjustable divider clock implementation
>> + */
>> +
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +
>> +/*
>> + * DOC: basic adjustable divider clock that cannot gate
>> + *
>> + * Traits of this clock:
>> + * prepare - clk_prepare only ensures that parents are prepared
>> + * enable - clk_enable only ensures that parents are enabled
>> + * rate - rate is adjustable.  clk->rate = parent->rate / divisor
>> + * parent - fixed parent.  No clk_set_parent support
>> + */
>> +
>> +#define to_clk_divider(_hw) container_of(_hw, struct clk_divider, hw)
>> +
>> +static unsigned long clk_divider_recalc_rate(struct clk_hw *hw,
>> +unsigned long parent_rate)
>> +{
>> +struct clk_divider *divider = to_clk_divider(hw);
>> +unsigned int div;
>> +unsigned long flags = 0;
>> +
>> +if (divider->lock)
>> +spin_lock_irqsave(divider->lock, flags);
>> +
>> +div = readl(divider->reg) >> divider->shift;
>> +div &= (1 << divider->width) - 1;
>> +
>> +if (divider->lock)
>> +spin_unlock_irqrestore(divider->lock, flags);
>> +
>> +if (!(divider->flags & CLK_DIVIDER_ONE_BASED))
>> +div++;
>> +
>> +return

Re: [PATCH v6 2/3] clk: introduce the common clock framework

2012-03-13 Thread Rob Herring
Mike,

On 03/10/2012 01:54 AM, Mike Turquette wrote:
> The common clock framework defines a common struct clk useful across
> most platforms as well as an implementation of the clk api that drivers
> can use safely for managing clocks.
> 
> The net result is consolidation of many different struct clk definitions
> and platform-specific clock framework implementations.
> 
> This patch introduces the common struct clk, struct clk_ops and an
> implementation of the well-known clock api in include/clk/clk.h.
> Platforms may define their own hardware-specific clock structure and
> their own clock operation callbacks, so long as it wraps an instance of
> struct clk_hw.
> 
> See Documentation/clk.txt for more details.
> 
> This patch is based on the work of Jeremy Kerr, which in turn was based
> on the work of Ben Herrenschmidt.
> 
> Signed-off-by: Mike Turquette 
> Signed-off-by: Mike Turquette 
> Cc: Russell King 
> Cc: Jeremy Kerr 
> Cc: Thomas Gleixner 
> Cc: Arnd Bergman 
> Cc: Paul Walmsley 
> Cc: Shawn Guo 
> Cc: Sascha Hauer 
> Cc: Richard Zhao 
> Cc: Saravana Kannan 
> Cc: Magnus Damm 
> Cc: Rob Herring 
> Cc: Mark Brown 
> Cc: Linus Walleij 
> Cc: Stephen Boyd 
> Cc: Amit Kucheria 
> Cc: Deepak Saxena 
> Cc: Grant Likely 
> Cc: Andrew Lunn 

snip

> +
> + /*
> +  * walk the list of orphan clocks and reparent any that are children of
> +  * this clock
> +  */
> + hlist_for_each_entry(orphan, tmp, &clk_orphan_list, child_node)

In __clk_init, this needs to be hlist_for_each_entry_safe as entries can
be removed.

Rob

___
linaro-dev mailing list
linaro-dev@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev


Re: [PATCH v6 3/3] clk: basic clock hardware types

2012-03-12 Thread Rob Herring
On 03/12/2012 03:58 PM, Turquette, Mike wrote:
> On Mon, Mar 12, 2012 at 1:18 PM, Rob Herring  wrote:
>> On 03/10/2012 01:54 AM, Mike Turquette wrote:
>>> Many platforms support simple gateable clocks, fixed-rate clocks,
>>> adjustable divider clocks and multi-parent multiplexer clocks.
>>>
>>> This patch introduces basic clock types for the above-mentioned hardware
>>> which share some common characteristics.
>>>
>>> Based on original work by Jeremy Kerr and contribution by Jamie Iles.
>>> Dividers and multiplexor clocks originally contributed by Richard Zhao &
>>> Sascha Hauer.
>>>

snip

>>> +static unsigned long clk_divider_recalc_rate(struct clk_hw *hw,
>>> + unsigned long parent_rate)
>>> +{
>>> + struct clk_divider *divider = to_clk_divider(hw);
>>> + unsigned int div;
>>> + unsigned long flags = 0;
>>> +
>>> + if (divider->lock)
>>> + spin_lock_irqsave(divider->lock, flags);
>>> +
>>> + div = readl(divider->reg) >> divider->shift;
>>> + div &= (1 << divider->width) - 1;
>>> +
>>> + if (divider->lock)
>>> + spin_unlock_irqrestore(divider->lock, flags);
>>
>> What are you locking against? You are only reading the register.
> 
> Hi Rob,
> 
> These register-level locks originally came in from the divider &
> multiplexer patches from Richard and Sascha and I'm sure they can give
> you more details than I.
> 
> Basically on their platform they have some 32-bits regs that have a
> lot of overlapping clock functions in them, such as enable/disable and
> adjusting a divider all in one reg.  Those operations are protected by
> different locks (enable spinlock and prepare mutex, respectively) so
> some synchronization mechanism is necessary.  On OMAP we don't use
> this since we have a billion registers that typically only map to one
> clock each.  I also wonder if having device type memory for the
> affected regions makes a this irrelevant on ARM... but that wouldn't
> matter for non-ARM architectures.  Just a thought.
> 

In fact, I pointed out that locking for a register access was needed in
an early version of this series as well. However, locking is only needed
if you are doing a read-mod-write on a field in a shared register or
reading from multiple registers. It makes no sense if you are *only*
reading a single shared register as is the case here. That's already an
atomic operation.

Rob


___
linaro-dev mailing list
linaro-dev@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev


Re: [PATCH v6 3/3] clk: basic clock hardware types

2012-03-12 Thread Rob Herring
On 03/10/2012 01:54 AM, Mike Turquette wrote:
> Many platforms support simple gateable clocks, fixed-rate clocks,
> adjustable divider clocks and multi-parent multiplexer clocks.
> 
> This patch introduces basic clock types for the above-mentioned hardware
> which share some common characteristics.
> 
> Based on original work by Jeremy Kerr and contribution by Jamie Iles.
> Dividers and multiplexor clocks originally contributed by Richard Zhao &
> Sascha Hauer.
> 
> Signed-off-by: Mike Turquette 
> Signed-off-by: Mike Turquette 
> Cc: Russell King 
> Cc: Jeremy Kerr 
> Cc: Thomas Gleixner 
> Cc: Arnd Bergman 
> Cc: Paul Walmsley 
> Cc: Shawn Guo 
> Cc: Sascha Hauer 
> Cc: Jamie Iles 
> Cc: Richard Zhao 
> Cc: Saravana Kannan 
> Cc: Magnus Damm 
> Cc: Rob Herring 
> Cc: Mark Brown 
> Cc: Linus Walleij 
> Cc: Stephen Boyd 
> Cc: Amit Kucheria 
> Cc: Deepak Saxena 
> Cc: Grant Likely 
> Cc: Andrew Lunn 

One issue with spinlocks below, but otherwise:

Reviewed-by: Rob Herring 

> ---
> Changes since v5:
>  * standardized the hw-specific locking in the basic clock types
>  * export the individual ops for each basic clock type
>  * improve registration for single-parent basic clocks (thanks Sascha)
>  * fixed bugs in gate clock's static initializers (thanks Andrew)
> 
>  drivers/clk/Makefile |3 +-
>  drivers/clk/clk-divider.c|  204 
> ++
>  drivers/clk/clk-fixed-rate.c |   82 +
>  drivers/clk/clk-gate.c   |  157 
>  drivers/clk/clk-mux.c|  123 +
>  include/linux/clk-private.h  |  124 +
>  include/linux/clk-provider.h |  127 ++
>  7 files changed, 819 insertions(+), 1 deletions(-)
>  create mode 100644 drivers/clk/clk-divider.c
>  create mode 100644 drivers/clk/clk-fixed-rate.c
>  create mode 100644 drivers/clk/clk-gate.c
>  create mode 100644 drivers/clk/clk-mux.c
> 
> diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile
> index ff362c4..1f736bc 100644
> --- a/drivers/clk/Makefile
> +++ b/drivers/clk/Makefile
> @@ -1,3 +1,4 @@
>  
>  obj-$(CONFIG_CLKDEV_LOOKUP)  += clkdev.o
> -obj-$(CONFIG_COMMON_CLK) += clk.o
> +obj-$(CONFIG_COMMON_CLK) += clk.o clk-fixed-rate.o clk-gate.o \
> +clk-mux.o clk-divider.o
> diff --git a/drivers/clk/clk-divider.c b/drivers/clk/clk-divider.c
> new file mode 100644
> index 000..c0c4e0b
> --- /dev/null
> +++ b/drivers/clk/clk-divider.c
> @@ -0,0 +1,204 @@
> +/*
> + * Copyright (C) 2011 Sascha Hauer, Pengutronix 
> + * Copyright (C) 2011 Richard Zhao, Linaro 
> + * Copyright (C) 2011-2012 Mike Turquette, Linaro Ltd 
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + * Adjustable divider clock implementation
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +/*
> + * DOC: basic adjustable divider clock that cannot gate
> + *
> + * Traits of this clock:
> + * prepare - clk_prepare only ensures that parents are prepared
> + * enable - clk_enable only ensures that parents are enabled
> + * rate - rate is adjustable.  clk->rate = parent->rate / divisor
> + * parent - fixed parent.  No clk_set_parent support
> + */
> +
> +#define to_clk_divider(_hw) container_of(_hw, struct clk_divider, hw)
> +
> +static unsigned long clk_divider_recalc_rate(struct clk_hw *hw,
> + unsigned long parent_rate)
> +{
> + struct clk_divider *divider = to_clk_divider(hw);
> + unsigned int div;
> + unsigned long flags = 0;
> +
> + if (divider->lock)
> + spin_lock_irqsave(divider->lock, flags);
> +
> + div = readl(divider->reg) >> divider->shift;
> + div &= (1 << divider->width) - 1;
> +
> + if (divider->lock)
> + spin_unlock_irqrestore(divider->lock, flags);

What are you locking against? You are only reading the register.

> +
> + if (!(divider->flags & CLK_DIVIDER_ONE_BASED))
> + div++;
> +
> + return parent_rate / div;
> +}
> +EXPORT_SYMBOL_GPL(clk_divider_recalc_rate);
> +
> +static int clk_divider_bestdiv(struct clk_hw *hw, unsigned long rate,
> + unsigned long *best_parent_rate)
> +{
> + struct clk_divider *divider = to_clk_divider(hw);
> + int i, bestdiv = 0;
> + unsigned long parent_rate, best = 0, now, maxdiv;
> +
> 

Re: [PATCH v2 1/4] mmc: omap_hsmmc: Convert hsmmc driver to use device tree

2012-03-08 Thread Rob Herring
On 03/07/2012 09:46 PM, Rajendra Nayak wrote:
> Hi Rob/Grant,
> 
> On Thursday 23 February 2012 05:31 PM, Rajendra Nayak wrote:
>> Define dt bindings for the ti-omap-hsmmc, and adapt
>> the driver to extract data (which was earlier passed as
>> platform_data) from device tree.
> 
> Any comments on these bindings for omap hsmmc? All the dependent
> patches/series on which this series was based have now made it to
> the respective -next of Mark and Chris.
> 

Acked-by: Rob Herring 

> 
>>
>> Signed-off-by: Rajendra Nayak
>> ---
>>   .../devicetree/bindings/mmc/ti-omap-hsmmc.txt  |   31 +
>>   drivers/mmc/host/omap_hsmmc.c  |   68
>> 
>>   2 files changed, 99 insertions(+), 0 deletions(-)
>>   create mode 100644
>> Documentation/devicetree/bindings/mmc/ti-omap-hsmmc.txt
>>
>> diff --git a/Documentation/devicetree/bindings/mmc/ti-omap-hsmmc.txt
>> b/Documentation/devicetree/bindings/mmc/ti-omap-hsmmc.txt
>> new file mode 100644
>> index 000..e4fa8f0
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/mmc/ti-omap-hsmmc.txt
>> @@ -0,0 +1,31 @@
>> +* TI Highspeed MMC host controller for OMAP
>> +
>> +The Highspeed MMC Host Controller on TI OMAP family
>> +provides an interface for MMC, SD, and SDIO types of memory cards.
>> +
>> +Required properties:
>> +- compatible:
>> + Should be "ti,omap2-hsmmc", for OMAP2/3 controllers
>> + Should be "ti,omap4-hsmmc", for OMAP4 controllers
>> +- ti,hwmods: Must be "mmc", n is controller instance starting 1
>> +- reg : should contain hsmmc registers location and length
>> +
>> +Optional properties:
>> +ti,dual-volt: boolean, supports dual voltage cards
>> +-supply: phandle to the regulator device tree node
>> +"supply-name" examples are "vmmc", "vmmc_aux" etc
>> +ti,bus-width: Number of data lines, default assumed is 1 if the
>> property is missing.
>> +cd-gpios: GPIOs for card detection
>> +wp-gpios: GPIOs for write protection
>> +ti,non-removable: non-removable slot (like eMMC)
>> +
>> +Example:
>> +mmc1: mmc@0x4809c000 {
>> +compatible = "ti,omap4-hsmmc";
>> +reg =<0x4809c000 0x400>;
>> +ti,hwmods = "mmc1";
>> +ti,dual-volt;
>> +ti,bus-width =<4>;
>> +vmmc-supply =<&vmmc>; /* phandle to regulator node */
>> +ti,non-removable;
>> +};
>> diff --git a/drivers/mmc/host/omap_hsmmc.c
>> b/drivers/mmc/host/omap_hsmmc.c
>> index 35f6dc1..0c93d58 100644
>> --- a/drivers/mmc/host/omap_hsmmc.c
>> +++ b/drivers/mmc/host/omap_hsmmc.c
>> @@ -26,6 +26,9 @@
>>   #include
>>   #include
>>   #include
>> +#include
>> +#include
>> +#include
>>   #include
>>   #include
>>   #include
>> @@ -1718,6 +1721,46 @@ static void omap_hsmmc_debugfs(struct mmc_host
>> *mmc)
>>
>>   #endif
>>
>> +#ifdef CONFIG_OF
>> +static struct omap_mmc_platform_data *of_get_hsmmc_pdata(struct
>> device *dev)
>> +{
>> +struct omap_mmc_platform_data *pdata;
>> +struct device_node *np = dev->of_node;
>> +u32 bus_width;
>> +
>> +pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL);
>> +if (!pdata)
>> +return NULL; /* out of memory */
>> +
>> +if (of_find_property(np, "ti,dual-volt", NULL))
>> +pdata->controller_flags |= OMAP_HSMMC_SUPPORTS_DUAL_VOLT;
>> +
>> +/* This driver only supports 1 slot */
>> +pdata->nr_slots = 1;
>> +pdata->slots[0].switch_pin = of_get_named_gpio(np, "cd-gpios", 0);
>> +pdata->slots[0].gpio_wp = of_get_named_gpio(np, "wp-gpios", 0);
>> +
>> +if (of_find_property(np, "ti,non-removable", NULL)) {
>> +pdata->slots[0].nonremovable = true;
>> +pdata->slots[0].no_regulator_off_init = true;
>> +}
>> +of_property_read_u32(np, "ti,bus-width",&bus_width);
>> +if (bus_width == 4)
>> +pdata->slots[0].caps |= MMC_CAP_4_BIT_DATA;
>> +else if (bus_width == 8)
>> +pdata->slots[0].caps |= MMC_CAP_8_BIT_DATA;
>> +return pdata;
>> +}
>> +#else
>> +static inline struct omap_mmc_platform_data
>> +*of_get_hsmmc_pdata(struct device *dev)
>> +{
>> +return NULL;
>> +}
>> +#endif
>> +
>> +st

Re: Single zImage and KVM

2012-03-06 Thread Rob Herring
On 03/06/2012 01:23 PM, Michael Hope wrote:
> On Wed, Mar 7, 2012 at 3:50 AM, Rob Herring  wrote:
>> On 03/04/2012 04:02 PM, Michael Hope wrote:
>>> I'd like to have one KVM kernel image which is suitable for the real
>>> hardware host and the virtio based guest.  The single zImage plus
>>> Device Tree work seem like a great way to do this.
>>>
>>> We're currently using the vexpress-a15 on a Fast Model as the host and
>>> a vexpress-a15 as the guest.  Device Tree support is required to
>>> describe the virtio-mmio devices.  As a bonus, the vexpress-a9 and
>>> vexpress-a15 are the same hardware with a different memory map and can
>>> help demonstrate the Device Tree support.
>>
>> Except LPAE and non-LPAE will be 2 different builds... At least you will
>> be able to run the same non-LPAE kernel build on both.
> 
> Hardware virtualisation requires LPAE and we're planning on LPAE in
> the guest to match.

Good, but both types of guests will be supported I presume.

>>> What are the plans for single zImage?  Where does the vexpress-a15 fit
>>> in with that?  Could I bump it to the front of the list?
>>
>> DT support for vexp A9 is going into 3.4 I believe. Pawel has been
>> working on it and can probably give details on A15 support. A single
>> kernel for these 2 platforms (and A7 as well) is much simpler than
>> getting to a single zImage in general (i.e. omap plus i.mx). But we'll
>> be a lot closer in 3.4.
> 
> Linus 3.4 or the Landing Team 3.4?  From other emails it seems that
> the work may be more on the packaging side.

Linus 3.4. I don't follow Linaro kernels, but sounds like support is
already there in the kernel at least.

> Where are the Device Trees hosted?

Generally, in the kernel in arch/arm/boot/dts.

Rob

___
linaro-dev mailing list
linaro-dev@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev


Re: Single zImage and KVM

2012-03-06 Thread Rob Herring
On 03/04/2012 04:02 PM, Michael Hope wrote:
> I'd like to have one KVM kernel image which is suitable for the real
> hardware host and the virtio based guest.  The single zImage plus
> Device Tree work seem like a great way to do this.
> 
> We're currently using the vexpress-a15 on a Fast Model as the host and
> a vexpress-a15 as the guest.  Device Tree support is required to
> describe the virtio-mmio devices.  As a bonus, the vexpress-a9 and
> vexpress-a15 are the same hardware with a different memory map and can
> help demonstrate the Device Tree support.

Except LPAE and non-LPAE will be 2 different builds... At least you will
be able to run the same non-LPAE kernel build on both.

> 
> What are the plans for single zImage?  Where does the vexpress-a15 fit
> in with that?  Could I bump it to the front of the list?

DT support for vexp A9 is going into 3.4 I believe. Pawel has been
working on it and can probably give details on A15 support. A single
kernel for these 2 platforms (and A7 as well) is much simpler than
getting to a single zImage in general (i.e. omap plus i.mx). But we'll
be a lot closer in 3.4.

Rob


___
linaro-dev mailing list
linaro-dev@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev


Re: [PATCH v5 1/9] cpuidle: Add commonly used functionality for consolidation

2012-02-28 Thread Rob Herring
On 02/28/2012 09:45 AM, Rob Lee wrote:
> Hey Mike,
> 
> On Mon, Feb 27, 2012 at 6:06 PM, Turquette, Mike  wrote:
>> On Sun, Feb 26, 2012 at 8:47 PM, Robert Lee  wrote:
>>> +/**
>>> + * cpuidle_enter_wrap - performing timekeeping and irq around enter 
>>> function
>>> + * @dev: pointer to a valid cpuidle_device object
>>> + * @drv: pointer to a valid cpuidle_driver object
>>> + * @index: index of the target cpuidle state.
>>> + */
>>> +static inline int cpuidle_wrap_enter(struct cpuidle_device *dev,
>>> +   struct cpuidle_driver *drv, int index,
>>> +   int (*enter)(struct cpuidle_device *dev,
>>> +   struct cpuidle_driver *drv, int 
>>> index))
>>> +{
>>> +   ktime_t time_start, time_end;
>>> +   s64 diff;
>>> +
>>> +   time_start = ktime_get();
>>> +
>>> +   index = enter(dev, drv, index);
>>> +
>>> +   time_end = ktime_get();
>>> +
>>> +   local_irq_enable();
>>> +
>>> +   diff = ktime_to_us(ktime_sub(time_end, time_start));
>>> +   if (diff > INT_MAX)
>>> +   diff = INT_MAX;
>>> +
>>> +   dev->last_residency = (int) diff;
>>> +
>>> +   return index;
>>> +}
>>
>> Any reason that this code is in the header?  Why not in cpuidle.c?
>>
> 
> Not a strong reason.  I thought making it an inline would introduce
> slightly less new execution when adding this code (realizing that
> there are function calls immediately after, so the only benefit is the
> reduce popping and pushing).  But it does require an extra copy of
> this code for any platform driver that does not enable
> en_core_tk_irqen and instead makes calls to it directly (like omap3).
> For this case, I don't think the inline implementation should add
> extra code from what exists today as it should simply replace the
> existing platform time keeping calls to a standard one defined by the
> core cpuidle.
> 
But you will have multiple copies of the inlined code if platforms do
use it. Or is it used only by the core cpuidle code? In that case, gcc
can automatically inline static functions.

It seems a bit long to inline and this isn't performance critical (at
least for the enter side).

Rob

> I don't have a strong preference with using the inline so if you or
> others can give your opinion on which method to use and why, I'd be
> glad to read it.
> 
>> Regards,
>> Mike
> 
> ___
> linux-arm-kernel mailing list
> linux-arm-ker...@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel


___
linaro-dev mailing list
linaro-dev@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev


Re: [PATCH] gic : check if there are pending interrupts

2012-02-24 Thread Rob Herring
On 02/24/2012 07:45 AM, Daniel Lezcano wrote:
> The following patch checks if there are pending interrupts on the gic.
> 
> This function is needed for example for the ux500 cpuidle driver.
> When the A9 cores and the gic are decoupled from the PRCMU, the idle
> routine has to check if an interrupt is pending on the gic before entering
> in retention mode.

That sounds racy. Soon as you check an interrupt can assert.

> 
> Signed-off-by: Daniel Lezcano 
> ---
>  arch/arm/common/gic.c   |   37 
> +++
>  arch/arm/include/asm/hardware/gic.h |2 +-
>  2 files changed, 38 insertions(+), 1 deletions(-)
> 
> diff --git a/arch/arm/common/gic.c b/arch/arm/common/gic.c
> index aa52699..2528094 100644
> --- a/arch/arm/common/gic.c
> +++ b/arch/arm/common/gic.c
> @@ -750,6 +750,43 @@ void gic_raise_softirq(const struct cpumask *mask, 
> unsigned int irq)
>  }
>  #endif
>  
> +/*
> + * gic_pending_irq - checks if there are pending interrupts on the gic
> + *
> + * Disabling an interrupt only disables the forwarding of the
> + * interrupt to any CPU interface. It does not prevent the interrupt
> + * from changing state, for example becoming pending, or active and
> + * pending if it is already active. For this reason, we have to check
> + * the interrupt is pending *and* is enabled.
> + *
> + * Returns true if there are pending and enabled interrupts, false
> + * otherwise.
> + */
> +bool gic_pending_irq(unsigned int gic_nr)

Seems like this should be solved with a generic interface and not gic
specific.

Would we ever need this for a secondary gic (gic_nr != 0)?

> +{
> + u32 pr; /* Pending register */
> + u32 er; /* Enable register */
> + void __iomem *dist_base;
> + int gic_irqs;
> + int i;
> +
> + BUG_ON(gic_nr >= MAX_GIC_NR);
> +
> + gic_irqs = gic_data[gic_nr].gic_irqs;
> + dist_base = gic_data_dist_base(&gic_data[gic_nr]);
> +
> + for (i = 0; i < DIV_ROUND_UP(gic_irqs, 32); i++) {

Wouldn't you want to skip PPIs if the CPU interface is disabled?

> +
> + pr = readl_relaxed(dist_base + GIC_DIST_PENDING_SET + i * 4);
> + er = readl_relaxed(dist_base + GIC_DIST_ENABLE_SET + i * 4);
> +

What if an interrupt is pending, but routed to a core which is not
getting put into low power state?

Rob

> + if (pr & er)
> + return true;
> + }
> +
> + return false;
> +}
> +
>  #ifdef CONFIG_OF
>  static int gic_cnt __initdata = 0;
>  
> diff --git a/arch/arm/include/asm/hardware/gic.h 
> b/arch/arm/include/asm/hardware/gic.h
> index 4b1ce6c..d198ac0 100644
> --- a/arch/arm/include/asm/hardware/gic.h
> +++ b/arch/arm/include/asm/hardware/gic.h
> @@ -45,7 +45,7 @@ void gic_secondary_init(unsigned int);
>  void gic_handle_irq(struct pt_regs *regs);
>  void gic_cascade_irq(unsigned int gic_nr, unsigned int irq);
>  void gic_raise_softirq(const struct cpumask *mask, unsigned int irq);
> -
> +bool gic_pending_irq(unsigned int gic_nr);
>  static inline void gic_init(unsigned int nr, int start,
>   void __iomem *dist , void __iomem *cpu)
>  {


___
linaro-dev mailing list
linaro-dev@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev


Re: [PATCH v4 3/6] clk: introduce the common clock framework

2012-01-12 Thread Rob Herring
On 01/12/2012 06:04 PM, Saravana Kannan wrote:
> On 01/04/2012 08:07 PM, Turquette, Mike wrote:
>> On Wed, Jan 4, 2012 at 6:11 PM, Rob Herring 
>> wrote:
>>> On 01/04/2012 07:01 PM, Turquette, Mike wrote:
>>>> On Wed, Jan 4, 2012 at 6:32 AM, Rob Herring 
>>>> wrote:
>>>>> On 01/03/2012 08:15 PM, Richard Zhao wrote:
>>>>>> On Fri, Dec 16, 2011 at 04:45:48PM -0800, Turquette, Mike wrote:
>>>>>>> On Wed, Dec 14, 2011 at 5:18 AM, Thomas
>>>>>>> Gleixner  wrote:
>>>>>>>> On Tue, 13 Dec 2011, Mike Turquette wrote:
>>>>>
>>>>> snip
>>>>>
>>>>>>>>> +/**
>>>>>>>>> + * clk_init - initialize the data structures in a struct clk
>>>>>>>>> + * @dev: device initializing this clk, placeholder for now
>>>>>>>>> + * @clk: clk being initialized
>>>>>>>>> + *
>>>>>>>>> + * Initializes the lists in struct clk, queries the hardware
>>>>>>>>> for the
>>>>>>>>> + * parent and rate and sets them both.  Adds the clk to the
>>>>>>>>> sysfs tree
>>>>>>>>> + * topology.
>>>>>>>>> + *
>>>>>>>>> + * Caller must populate clk->name and clk->flags before calling
>>>>>>>>
>>>>>>>> I'm not too happy about this construct. That leaves struct clk
>>>>>>>> and its
>>>>>>>> members exposed to the world instead of making it a real opaque
>>>>>>>> cookie. I know from my own painful experience, that this will
>>>>>>>> lead to
>>>>>>>> random fiddling in that data structure in drivers and arch code
>>>>>>>> just
>>>>>>>> because the core code has a shortcoming.
>>>>>>>>
>>>>>>>> Why can't we make struct clk a real cookie and confine the data
>>>>>>>> structure to the core code ?
>>>>>>>>
>>>>>>>> That would change the init call to something like:
>>>>>>>>
>>>>>>>> struct clk *clk_init(struct device *dev, const struct clk_hw *hw,
>>>>>>>>  struct clk *parent)
>>>>>>>>
>>>>>>>> And have:
>>>>>>>> struct clk_hw {
>>>>>>>>struct clk_hw_ops *ops;
>>>>>>>>const char*name;
>>>>>>>>unsigned long flags;
>>>>>>>> };
>>>>>>>>
>>>>>>>> Implementers can do:
>>>>>>>> struct my_clk_hw {
>>>>>>>>struct clk_hwhw;
>>>>>>>>mydata;
>>>>>>>> };
>>>>>>>>
>>>>>>>> And then change the clk ops callbacks to take struct clk_hw * as an
>>>>>>>> argument.
>>>>>> We have to define static clocks before we adopt DT binding.
>>>>>> If clk is opaque and allocate memory in clk core, it'll make hard
>>>>>> to define static clocks. And register/init will pass a long parameter
>>>>>> list.
>>>>>
>>>>> DT is not a prerequisite for having dynamically created clocks. You
>>>>> can
>>>>> make clock init dynamic without DT.
>>>>
>>>> Agreed.
>>>>
>>>>> What data goes in struct clk vs. struct clk_hw could change over time.
>>>>> So perhaps we can start with some data in clk_hw and plan to move
>>>>> it to
>>>>> struct clk later. Even if almost everything ends up in clk_hw
>>>>> initially,
>>>>> at least the structure is in place to have common, core-only data
>>>>> separate from platform data.
>>>>
>>>> What is the point of this?
>>>
>>> To have a way forward. It would be nice to have a clk infrastructure
>>> before I retire...
>>
>> Haha, agreed.
>>
>>>
>>>> The original clk_hw was defined simply as:
>>>>
>>>> struct clk_hw {
>>>>  struct clk *clk;
>>>> };
>>>>
>>>> It's 

Re: [PATCH v4 3/6] clk: introduce the common clock framework

2012-01-04 Thread Rob Herring
On 01/04/2012 07:01 PM, Turquette, Mike wrote:
> On Wed, Jan 4, 2012 at 6:32 AM, Rob Herring  wrote:
>> On 01/03/2012 08:15 PM, Richard Zhao wrote:
>>> On Fri, Dec 16, 2011 at 04:45:48PM -0800, Turquette, Mike wrote:
>>>> On Wed, Dec 14, 2011 at 5:18 AM, Thomas Gleixner  
>>>> wrote:
>>>>> On Tue, 13 Dec 2011, Mike Turquette wrote:
>>
>> snip
>>
>>>>>> +/**
>>>>>> + * clk_init - initialize the data structures in a struct clk
>>>>>> + * @dev: device initializing this clk, placeholder for now
>>>>>> + * @clk: clk being initialized
>>>>>> + *
>>>>>> + * Initializes the lists in struct clk, queries the hardware for the
>>>>>> + * parent and rate and sets them both.  Adds the clk to the sysfs tree
>>>>>> + * topology.
>>>>>> + *
>>>>>> + * Caller must populate clk->name and clk->flags before calling
>>>>>
>>>>> I'm not too happy about this construct. That leaves struct clk and its
>>>>> members exposed to the world instead of making it a real opaque
>>>>> cookie. I know from my own painful experience, that this will lead to
>>>>> random fiddling in that data structure in drivers and arch code just
>>>>> because the core code has a shortcoming.
>>>>>
>>>>> Why can't we make struct clk a real cookie and confine the data
>>>>> structure to the core code ?
>>>>>
>>>>> That would change the init call to something like:
>>>>>
>>>>> struct clk *clk_init(struct device *dev, const struct clk_hw *hw,
>>>>> struct clk *parent)
>>>>>
>>>>> And have:
>>>>> struct clk_hw {
>>>>>   struct clk_hw_ops *ops;
>>>>>   const char*name;
>>>>>   unsigned long flags;
>>>>> };
>>>>>
>>>>> Implementers can do:
>>>>> struct my_clk_hw {
>>>>>   struct clk_hwhw;
>>>>>   mydata;
>>>>> };
>>>>>
>>>>> And then change the clk ops callbacks to take struct clk_hw * as an
>>>>> argument.
>>> We have to define static clocks before we adopt DT binding.
>>> If clk is opaque and allocate memory in clk core, it'll make hard
>>> to define static clocks. And register/init will pass a long parameter
>>> list.
>>
>> DT is not a prerequisite for having dynamically created clocks. You can
>> make clock init dynamic without DT.
> 
> Agreed.
> 
>> What data goes in struct clk vs. struct clk_hw could change over time.
>> So perhaps we can start with some data in clk_hw and plan to move it to
>> struct clk later. Even if almost everything ends up in clk_hw initially,
>> at least the structure is in place to have common, core-only data
>> separate from platform data.
> 
> What is the point of this?

To have a way forward. It would be nice to have a clk infrastructure
before I retire...

> The original clk_hw was defined simply as:
> 
> struct clk_hw {
> struct clk *clk;
> };
> 
> It's only purpose in life was as a handle for navigation between the
> opaque struct clk and the hardware-specific struct my_clk_hw.  struct
> clk_hw is defined in clk.h and everyone can see it.  If we're suddenly
> OK putting clk data in this structure then why bother with an opaque
> struct clk at all?
> 
>> What is the actual data you need to be static and accessible to the
>> platform code? A ptr to parent clk is the main thing I've seen for
>> static initialization. So make the parent ptr be struct clk_hw* and
>> allow the platforms to access.
> 
> To answer your question on what data we're trying to expose: platform
> code commonly needs the parent pointer and the clk rate (and by
> extension, the rate of the parent).  For debug/error prints it is also
> nice to have the clk name.  Generic clk flags are also conceivably
> something that platform code might want.

I agree with the need to have the parent and flags from a static init
perspective. There's not really a good reason the others can't be
accessed thru accessors though.

> I'd like to spin the question around: if we're OK exposing some stuff
> (in your example above, the parent pointer), then what clk data are
> you trying to hide?

Well, everything from drivers which the current clk implementations do
hide. Catching abuse in with drivers coming in from all different trees
and lists will be impossible.

For platform code it is more fuzzy. I don't think platform code should
be allowed to muck with prepare/enable counts for example.

Rob

> 
> Regards,
> Mike
> 
>>
>> Rob


___
linaro-dev mailing list
linaro-dev@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev


Re: [PATCH v4 3/6] clk: introduce the common clock framework

2012-01-04 Thread Rob Herring
On 01/03/2012 08:15 PM, Richard Zhao wrote:
> On Fri, Dec 16, 2011 at 04:45:48PM -0800, Turquette, Mike wrote:
>> On Wed, Dec 14, 2011 at 5:18 AM, Thomas Gleixner  wrote:
>>> On Tue, 13 Dec 2011, Mike Turquette wrote:

snip

 +/**
 + * clk_init - initialize the data structures in a struct clk
 + * @dev: device initializing this clk, placeholder for now
 + * @clk: clk being initialized
 + *
 + * Initializes the lists in struct clk, queries the hardware for the
 + * parent and rate and sets them both.  Adds the clk to the sysfs tree
 + * topology.
 + *
 + * Caller must populate clk->name and clk->flags before calling
>>>
>>> I'm not too happy about this construct. That leaves struct clk and its
>>> members exposed to the world instead of making it a real opaque
>>> cookie. I know from my own painful experience, that this will lead to
>>> random fiddling in that data structure in drivers and arch code just
>>> because the core code has a shortcoming.
>>>
>>> Why can't we make struct clk a real cookie and confine the data
>>> structure to the core code ?
>>>
>>> That would change the init call to something like:
>>>
>>> struct clk *clk_init(struct device *dev, const struct clk_hw *hw,
>>> struct clk *parent)
>>>
>>> And have:
>>> struct clk_hw {
>>>   struct clk_hw_ops *ops;
>>>   const char*name;
>>>   unsigned long flags;
>>> };
>>>
>>> Implementers can do:
>>> struct my_clk_hw {
>>>   struct clk_hwhw;
>>>   mydata;
>>> };
>>>
>>> And then change the clk ops callbacks to take struct clk_hw * as an
>>> argument.
> We have to define static clocks before we adopt DT binding.
> If clk is opaque and allocate memory in clk core, it'll make hard
> to define static clocks. And register/init will pass a long parameter
> list.

DT is not a prerequisite for having dynamically created clocks. You can
make clock init dynamic without DT.

What data goes in struct clk vs. struct clk_hw could change over time.
So perhaps we can start with some data in clk_hw and plan to move it to
struct clk later. Even if almost everything ends up in clk_hw initially,
at least the structure is in place to have common, core-only data
separate from platform data.

What is the actual data you need to be static and accessible to the
platform code? A ptr to parent clk is the main thing I've seen for
static initialization. So make the parent ptr be struct clk_hw* and
allow the platforms to access.

Rob

___
linaro-dev mailing list
linaro-dev@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev


Re: [PATCH V3 4/7] cpufreq: add generic cpufreq driver

2011-12-20 Thread Rob Herring
On 12/19/2011 07:59 PM, Richard Zhao wrote:
> On Mon, Dec 19, 2011 at 09:00:44AM -0600, Rob Herring wrote:
>> On 12/19/2011 08:39 AM, Jamie Iles wrote:
>>> On Mon, Dec 19, 2011 at 10:19:29PM +0800, Richard Zhao wrote:
>>>> On Mon, Dec 19, 2011 at 10:05:12AM +, Jamie Iles wrote:
>>>>> Hi Richard,
>>>>>
>>>>> On Mon, Dec 19, 2011 at 11:21:40AM +0800, Richard Zhao wrote:
>>>>>> It support single core and multi-core ARM SoCs. But currently it assume
>>>>>> all cores share the same frequency and voltage.
>>>>>>
>>>>>> Signed-off-by: Richard Zhao 
>>>>>> ---
>>>>>>  .../devicetree/bindings/cpufreq/generic-cpufreq|7 +
>>>>>>  drivers/cpufreq/Kconfig|8 +
>>>>>>  drivers/cpufreq/Makefile   |2 +
>>>>>>  drivers/cpufreq/generic-cpufreq.c  |  251 
>>>>>> 
>>>>>>  4 files changed, 268 insertions(+), 0 deletions(-)
>>>>>>  create mode 100644 
>>>>>> Documentation/devicetree/bindings/cpufreq/generic-cpufreq
>>>>>>  create mode 100644 drivers/cpufreq/generic-cpufreq.c
>>>>>>
>>>>>> diff --git a/Documentation/devicetree/bindings/cpufreq/generic-cpufreq 
>>>>>> b/Documentation/devicetree/bindings/cpufreq/generic-cpufreq
>>>>>> new file mode 100644
>>>>>> index 000..15dd780
>>>>>> --- /dev/null
>>>>>> +++ b/Documentation/devicetree/bindings/cpufreq/generic-cpufreq
>>>>>> @@ -0,0 +1,7 @@
>>>>>> +Generic cpufreq driver
>>>>>> +
>>>>>> +Required properties in /cpus/cpu@0:
>>>>>> +- compatible : "generic-cpufreq"
>>>>>
>>>>> I'm not convinced this is the best way to do this.  By requiring a 
>>>>> generic-cpufreq compatible string we're encoding Linux driver 
>>>>> information into the hardware description.  The only way I can see to 
>>>>> avoid this is to provide a generic_clk_cpufreq_init() function that 
>>>>> platforms can call in their machine init code to use the driver.
>>
>> Agreed on the compatible string.
> Assume you reject to use compatible string.
>> It's putting Linux specifics into DT.
>>
>> You could flip this around and have the module make a call into the
>> kernel to determine whether to initialize or not. Then platforms could
>> set a flag to indicate this.
> Could you make it more clear? kernel global variable, macro, or function?

Any of those. Of course, direct access to variables across modules is
discouraged, so it would probably be a function that checks a variable.

> - Following your idea, I think, we can add in driver/cpufreq/cpufreq.c:
> int (*clk_reg_cpufreq_get_op_table) (struct op_table *tbl, int *size);
> SoC code set the callback. If it's NULL, driver will exit. We can get rid
> of DT. It'll make cpufreq core dirty, but it's the only file built-in.

But aren't you getting the operating points from the DT? Then you don't
want to put this code into each platform.

> 
> - Drop module support. SoC call generic_clk_cpufreq_init as Jamie said.
> 
>>
>>>> It'll prevent the driver from being a kernel module.
>>>
>>> Hmm, that's not very nice either!  I guess you _could_ add an 
>>> of_machine_is_compatible() check against a list of compatible machines 
>>> in the driver but that feels a little gross.  Hopefully Rob or Grant 
>>> have a good alternative!
>>>
>>
>> What does cpufreq core do if multiple drivers are registered?
> current cpufreq core only support one cpufreq_driver. Others will fail
> except the first time.

Then whoever gets there first wins. Make your driver register late and
if someone doesn't want to use it they can register a custom driver earlier.

Rob

>> Perhaps a
>> ranking is needed and this would only get enabled if there are no other
>> drivers and other conditions like having the clock "cpu" present are met.
> We'd better keep cpufreq core simple. For this driver, register cpufreq_driver
> is the last thing after checking all conditions.
> 
>>
>> Rob
>>
>>>> Hi Grant & Rob,
>>>>
>>>> Could you comment?
>>>>
>>>>>
>>>>>> +- cpu-freqs : cpu frequency points it support
>>>&g

Re: [PATCH V3 4/7] cpufreq: add generic cpufreq driver

2011-12-19 Thread Rob Herring
On 12/19/2011 08:39 AM, Jamie Iles wrote:
> On Mon, Dec 19, 2011 at 10:19:29PM +0800, Richard Zhao wrote:
>> On Mon, Dec 19, 2011 at 10:05:12AM +, Jamie Iles wrote:
>>> Hi Richard,
>>>
>>> On Mon, Dec 19, 2011 at 11:21:40AM +0800, Richard Zhao wrote:
 It support single core and multi-core ARM SoCs. But currently it assume
 all cores share the same frequency and voltage.

 Signed-off-by: Richard Zhao 
 ---
  .../devicetree/bindings/cpufreq/generic-cpufreq|7 +
  drivers/cpufreq/Kconfig|8 +
  drivers/cpufreq/Makefile   |2 +
  drivers/cpufreq/generic-cpufreq.c  |  251 
 
  4 files changed, 268 insertions(+), 0 deletions(-)
  create mode 100644 
 Documentation/devicetree/bindings/cpufreq/generic-cpufreq
  create mode 100644 drivers/cpufreq/generic-cpufreq.c

 diff --git a/Documentation/devicetree/bindings/cpufreq/generic-cpufreq 
 b/Documentation/devicetree/bindings/cpufreq/generic-cpufreq
 new file mode 100644
 index 000..15dd780
 --- /dev/null
 +++ b/Documentation/devicetree/bindings/cpufreq/generic-cpufreq
 @@ -0,0 +1,7 @@
 +Generic cpufreq driver
 +
 +Required properties in /cpus/cpu@0:
 +- compatible : "generic-cpufreq"
>>>
>>> I'm not convinced this is the best way to do this.  By requiring a 
>>> generic-cpufreq compatible string we're encoding Linux driver 
>>> information into the hardware description.  The only way I can see to 
>>> avoid this is to provide a generic_clk_cpufreq_init() function that 
>>> platforms can call in their machine init code to use the driver.

Agreed on the compatible string. It's putting Linux specifics into DT.

You could flip this around and have the module make a call into the
kernel to determine whether to initialize or not. Then platforms could
set a flag to indicate this.

>> It'll prevent the driver from being a kernel module.
> 
> Hmm, that's not very nice either!  I guess you _could_ add an 
> of_machine_is_compatible() check against a list of compatible machines 
> in the driver but that feels a little gross.  Hopefully Rob or Grant 
> have a good alternative!
> 

What does cpufreq core do if multiple drivers are registered? Perhaps a
ranking is needed and this would only get enabled if there are no other
drivers and other conditions like having the clock "cpu" present are met.

Rob

>> Hi Grant & Rob,
>>
>> Could you comment?
>>
>>>
 +- cpu-freqs : cpu frequency points it support
 +- cpu-volts : cpu voltages required by the frequency point at the same 
 index
 +- trans-latency :  transition_latency
 diff --git a/drivers/cpufreq/Kconfig b/drivers/cpufreq/Kconfig
 index e24a2a1..216eecd 100644
 --- a/drivers/cpufreq/Kconfig
 +++ b/drivers/cpufreq/Kconfig
 @@ -179,6 +179,14 @@ config CPU_FREQ_GOV_CONSERVATIVE
  
  If in doubt, say N.
  
 +config GENERIC_CPUFREQ_DRIVER
 +  bool "Generic cpufreq driver using clock/regulator/devicetree"
 +  help
 +This adds generic CPUFreq driver. It assumes all
 +cores of the CPU share the same clock and voltage.
 +
 +If in doubt, say N.
>>>
>>> I think this needs dependencies on HAVE_CLK, OF and REGULATOR.
>> right, Thanks. I can not check clk before generic clock framework
>> come in.
>> Added:
>>  depends on OF && REGULATOR
>>  select CPU_FREQ_TABLE
> 
> You can still use HAVE_CLK.  That symbol has been around for ages and 
> any platform implementing the clk API should select it so it's fine to 
> depend on it even before there is a generic struct clk.
> 
> Jamie

___
linaro-dev mailing list
linaro-dev@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev


Re: [PATCH v3 0/4] OMAP serial device tree support

2011-12-16 Thread Rob Herring
Tony,

On 12/16/2011 03:57 PM, Tony Lindgren wrote:
> Rob,
> 
> * Rob Herring  [111214 05:16]:
>> On 12/14/2011 05:55 AM, Rajendra Nayak wrote:
>>> v3 is rebased on top of the latest serial runtime
>>> patches[1] and boot tested with/without DT on OMAP4
>>> SDP and OMAP4 Panda boards.
>>>
>>> Patches can be found here..
>>> git://gitorious.org/omap-pm/linux.git for-dt/serial
>>>
>>> I also had to pull in a fix[2] for DT testing (already in linux-omap
>>> master) which was missing as the serial runtime branch[1]
>>> was based on an older master commit.
>>>
>>> Changes in v3:
>>> -1- Rebased on latest serial runtime patches
>>> -2- Minor typr fixes
>>>
>>> Changes in v2:
>>> -1- Got rid of binding to define which uart is console
>>> -2- Added checks to default clock speed to 48Mhz
>>> -3- Added compatible for each OMAP family
>>> -4- Used of_alias_get_id to populate port.line
>>>
>>> [1] git://gitorious.org/runtime_3-0/runtime_3-0.git 
>>> for_3_3/lo_rc4_uartruntime
>>> [2] http://www.spinics.net/lists/arm-kernel/msg150751.html
>>>
>>> Rajendra Nayak (4):
>>>   omap-serial: Get rid of all pdev->id usage
>>>   omap-serial: Use default clock speed (48Mhz) if not specified
>>>   omap-serial: Add minimal device tree support
>>>   ARM: omap: pass minimal SoC/board data for UART from dt
>>>
>>>  .../devicetree/bindings/serial/omap_serial.txt |   10 +++
>>>  arch/arm/boot/dts/omap3.dtsi   |   31 
>>>  arch/arm/boot/dts/omap4.dtsi   |   28 +++
>>>  arch/arm/mach-omap2/board-generic.c|1 -
>>>  drivers/tty/serial/omap-serial.c   |   80 
>>> +++
>>>  5 files changed, 132 insertions(+), 18 deletions(-)
>>>  create mode 100644 Documentation/devicetree/bindings/serial/omap_serial.txt
>>
>> Looks good. For the series:
>>
>> Reviewed-by: Rob Herring 
> 
> Care to check if your Reviewed-by can also be applied
> to the additional patch "[PATCH] arm/dts: Add minimal device
> tree support for omap2420 and omap2430" that's needed
> to keep serial port working for omap2 with this series?

It looks fine.

Rob

___
linaro-dev mailing list
linaro-dev@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev


Re: [PATCH V2 1/4] cpufreq: add arm soc generic cpufreq driver

2011-12-16 Thread Rob Herring
On 12/16/2011 04:30 AM, Richard Zhao wrote:
> It support single core and multi-core ARM SoCs. But it assume
> all cores share the same frequency and voltage.
> 
> Signed-off-by: Richard Zhao 
> ---
>  drivers/cpufreq/Kconfig.arm   |8 ++
>  drivers/cpufreq/Makefile  |1 +
>  drivers/cpufreq/arm-cpufreq.c |  269 
> +
>  3 files changed, 278 insertions(+), 0 deletions(-)
>  create mode 100644 drivers/cpufreq/arm-cpufreq.c
> 

What makes this specific to ARM and not a generic DT + clk api +
regulator api driver?

Also, you need documentation of the DT bindings.

Rob

> diff --git a/drivers/cpufreq/Kconfig.arm b/drivers/cpufreq/Kconfig.arm
> index 72a0044..a0f7d35 100644
> --- a/drivers/cpufreq/Kconfig.arm
> +++ b/drivers/cpufreq/Kconfig.arm
> @@ -2,6 +2,14 @@
>  # ARM CPU Frequency scaling drivers
>  #
>  
> +config ARM_GENERIC_CPUFREQ
> + bool "ARM generic"
> + help
> +   This adds ARM generic CPUFreq driver. It assumes all
> +   cores of the SoC share the same clock and voltage.
> +
> +   If in doubt, say N.
> +
>  config ARM_S3C64XX_CPUFREQ
>   bool "Samsung S3C64XX"
>   depends on CPU_S3C6410
> diff --git a/drivers/cpufreq/Makefile b/drivers/cpufreq/Makefile
> index ce75fcb..6ccf02d 100644
> --- a/drivers/cpufreq/Makefile
> +++ b/drivers/cpufreq/Makefile
> @@ -39,6 +39,7 @@ obj-$(CONFIG_X86_CPUFREQ_NFORCE2)   += cpufreq-nforce2.o
>  
>  
> ##
>  # ARM SoC drivers
> +obj-$(CONFIG_ARM_GENERIC_CPUFREQ)+= arm-cpufreq.o
>  obj-$(CONFIG_UX500_SOC_DB8500)   += db8500-cpufreq.o
>  obj-$(CONFIG_ARM_S3C64XX_CPUFREQ)+= s3c64xx-cpufreq.o
>  obj-$(CONFIG_ARM_S5PV210_CPUFREQ)+= s5pv210-cpufreq.o
> diff --git a/drivers/cpufreq/arm-cpufreq.c b/drivers/cpufreq/arm-cpufreq.c
> new file mode 100644
> index 000..e4d20da
> --- /dev/null
> +++ b/drivers/cpufreq/arm-cpufreq.c
> @@ -0,0 +1,269 @@
> +/*
> + * Copyright (C) 2011 Freescale Semiconductor, Inc.
> + */
> +
> +/*
> + * The code contained herein is licensed under the GNU General Public
> + * License. You may obtain a copy of the GNU General Public License
> + * Version 2 or later at the following locations:
> + *
> + * http://www.opensource.org/licenses/gpl-license.html
> + * http://www.gnu.org/copyleft/gpl.html
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +static u32 *cpu_freqs; /* HZ */
> +static u32 *cpu_volts; /* uV */
> +static u32 trans_latency; /* ns */
> +static int cpu_op_nr;
> +
> +static DEFINE_PER_CPU(unsigned long, l_p_j_ref);
> +static unsigned long l_p_j_ref_freq;
> +
> +static struct clk *cpu_clk;
> +static struct regulator *cpu_reg;
> +static struct cpufreq_frequency_table *arm_freq_table;
> +
> +static int set_cpu_freq(unsigned long freq, int index, int higher)
> +{
> + int ret = 0;
> +
> + if (higher && cpu_reg)
> + regulator_set_voltage(cpu_reg,
> + cpu_volts[index], cpu_volts[index]);
> +
> + ret = clk_set_rate(cpu_clk, freq);
> + if (ret != 0) {
> + printk(KERN_DEBUG "cannot set CPU clock rate\n");
> + return ret;
> + }
> +
> + if (!higher && cpu_reg)
> + regulator_set_voltage(cpu_reg,
> + cpu_volts[index], cpu_volts[index]);
> +
> + return ret;
> +}
> +
> +static int arm_verify_speed(struct cpufreq_policy *policy)
> +{
> + return cpufreq_frequency_table_verify(policy, arm_freq_table);
> +}
> +
> +static unsigned int arm_get_speed(unsigned int cpu)
> +{
> + return clk_get_rate(cpu_clk) / 1000;
> +}
> +
> +static int arm_set_target(struct cpufreq_policy *policy,
> +   unsigned int target_freq, unsigned int relation)
> +{
> + struct cpufreq_freqs freqs;
> + unsigned long freq_Hz;
> + int cpu;
> + int ret = 0;
> + unsigned int index;
> +
> + cpufreq_frequency_table_target(policy, arm_freq_table,
> + target_freq, relation, &index);
> + freq_Hz = clk_round_rate(cpu_clk, cpu_freqs[index]);
> + freq_Hz = freq_Hz ? freq_Hz : cpu_freqs[index];
> + freqs.old = clk_get_rate(cpu_clk) / 1000;
> + freqs.new = clk_round_rate(cpu_clk, cpu_freqs[index]);
> + freqs.new = freq_Hz / 1000;
> + freqs.flags = 0;
> +
> + if (freqs.old == freqs.new)
> + return 0;
> +
> + for_each_possible_cpu(cpu) {
> + freqs.cpu = cpu;
> + cpufreq_notify_transition(&freqs, CPUFREQ_PRECHANGE);
> + }
> +
> + ret = set_cpu_freq(freq_Hz, index, (freqs.new > freqs.old));
> +
> +#ifdef CONFIG_SMP
> + /* loops_per_jiffy is not updated by the cpufreq core for SMP systems.
> +  * So update it for all CPUs.
> +  */
> + for_each_possible_cpu(cpu)
> + per_cpu(cpu_data, cpu).loops_per_jiffy =
> + cpufreq_scale(per_cpu(l_p_j_ref, c

Re: [PATCH v3 0/4] OMAP serial device tree support

2011-12-14 Thread Rob Herring
On 12/14/2011 05:55 AM, Rajendra Nayak wrote:
> v3 is rebased on top of the latest serial runtime
> patches[1] and boot tested with/without DT on OMAP4
> SDP and OMAP4 Panda boards.
> 
> Patches can be found here..
> git://gitorious.org/omap-pm/linux.git for-dt/serial
> 
> I also had to pull in a fix[2] for DT testing (already in linux-omap
> master) which was missing as the serial runtime branch[1]
> was based on an older master commit.
> 
> Changes in v3:
> -1- Rebased on latest serial runtime patches
> -2- Minor typr fixes
> 
> Changes in v2:
> -1- Got rid of binding to define which uart is console
> -2- Added checks to default clock speed to 48Mhz
> -3- Added compatible for each OMAP family
> -4- Used of_alias_get_id to populate port.line
> 
> [1] git://gitorious.org/runtime_3-0/runtime_3-0.git for_3_3/lo_rc4_uartruntime
> [2] http://www.spinics.net/lists/arm-kernel/msg150751.html
> 
> Rajendra Nayak (4):
>   omap-serial: Get rid of all pdev->id usage
>   omap-serial: Use default clock speed (48Mhz) if not specified
>   omap-serial: Add minimal device tree support
>   ARM: omap: pass minimal SoC/board data for UART from dt
> 
>  .../devicetree/bindings/serial/omap_serial.txt |   10 +++
>  arch/arm/boot/dts/omap3.dtsi   |   31 
>  arch/arm/boot/dts/omap4.dtsi   |   28 +++
>  arch/arm/mach-omap2/board-generic.c|1 -
>  drivers/tty/serial/omap-serial.c   |   80 +++
>  5 files changed, 132 insertions(+), 18 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/serial/omap_serial.txt

Looks good. For the series:

Reviewed-by: Rob Herring 


___
linaro-dev mailing list
linaro-dev@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev


Re: [PATCH v2 0/4] OMAP serial device tree support

2011-11-28 Thread Rob Herring
On 11/28/2011 12:31 AM, Greg KH wrote:
> On Mon, Nov 28, 2011 at 11:36:56AM +0530, Rajendra Nayak wrote:
>> On Sunday 27 November 2011 09:06 AM, Greg KH wrote:
>>> On Tue, Nov 22, 2011 at 07:14:12PM +0530, Rajendra Nayak wrote:
 v2 is based on the latest omap-serial runtime patches, which
 can be found here[1]

 The series passes minimal data that allows serial console
 boot, with UART's initialised from device tree.
 However some of low power support for UART and remote
 wakeup needs more work.
 Boot tested on OMAP4 panda and OMAP4 sdp boards.

 Patches can be found here..
 git://gitorious.org/omap-pm/linux.git for-dt/serial
>>>
>>> What tree are these going to go through, some device tree one, or do you
>>> want me to take the serial driver patches?
>>
>> Hi Greg,
>>
>> These patches have a dependency on the omap-serial runtime series.
>> So once that makes it in, through the serial driver tree, and once we
>> have the necessary Acks from the Device Tree maintainers on the relevant
>> patches from this series, these could go through the serial driver tree
>> as well.
> 
> Then all of these should probably go through the device tree tree.

Generally, DT related driver changes go in thru the respective driver trees.

Rob

___
linaro-dev mailing list
linaro-dev@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev


Re: [PATCH v2 4/4] ARM: omap: pass minimal SoC/board data for UART from dt

2011-11-28 Thread Rob Herring
On 11/22/2011 07:44 AM, Rajendra Nayak wrote:
> Pass minimal data needed for console boot, from dt, for
> OMAP4 panda/sdp and OMAP3 beagle boards, and get rid of the
> static initialization from generic board file.
> 
> Signed-off-by: Rajendra Nayak 

Acked-by: Rob Herring 

Rob
> ---
>  arch/arm/boot/dts/omap3.dtsi|   31 +++
>  arch/arm/boot/dts/omap4.dtsi|   28 
>  arch/arm/mach-omap2/board-generic.c |1 -
>  3 files changed, 59 insertions(+), 1 deletions(-)
> 
> diff --git a/arch/arm/boot/dts/omap3.dtsi b/arch/arm/boot/dts/omap3.dtsi
> index d202bb5..216c331 100644
> --- a/arch/arm/boot/dts/omap3.dtsi
> +++ b/arch/arm/boot/dts/omap3.dtsi
> @@ -13,6 +13,13 @@
>  / {
>   compatible = "ti,omap3430", "ti,omap3";
>  
> + aliases {
> + serial0 = &uart1;
> + serial1 = &uart2;
> + serial2 = &uart3;
> + serial3 = &uart4;
> + };
> +
>   cpus {
>   cpu@0 {
>   compatible = "arm,cortex-a8";
> @@ -59,5 +66,29 @@
>   interrupt-controller;
>   #interrupt-cells = <1>;
>   };
> +
> + uart1: serial@0x4806a000 {
> + compatible = "ti,omap3-uart";
> + ti,hwmods = "uart1";
> + clock-frequency = <4800>;
> + };
> +
> + uart2: serial@0x4806c000 {
> + compatible = "ti,omap3-uart";
> + ti,hwmods = "uart2";
> + clock-frequency = <4800>;
> + };
> +
> + uart3: serial@0x4902 {
> + compatible = "ti,omap3-uart";
> + ti,hwmods = "uart3";
> + clock-frequency = <4800>;
> + };
> +
> + uart4: serial@0x49042000 {
> + compatible = "ti,omap3-uart";
> + ti,hwmods = "uart4";
> + clock-frequency = <4800>;
> + };
>   };
>  };
> diff --git a/arch/arm/boot/dts/omap4.dtsi b/arch/arm/boot/dts/omap4.dtsi
> index 4c61c82..e8fe75f 100644
> --- a/arch/arm/boot/dts/omap4.dtsi
> +++ b/arch/arm/boot/dts/omap4.dtsi
> @@ -21,6 +21,10 @@
>   interrupt-parent = <&gic>;
>  
>   aliases {
> + serial0 = &uart1;
> + serial1 = &uart2;
> + serial2 = &uart3;
> + serial3 = &uart4;
>   };
>  
>   cpus {
> @@ -99,5 +103,29 @@
>   reg = <0x48241000 0x1000>,
> <0x48240100 0x0100>;
>   };
> +
> + uart1: serial@0x4806a000 {
> + compatible = "ti,omap4-uart";
> + ti,hwmods = "uart1";
> + clock-frequency = <4800>;
> + };
> +
> + uart2: serial@0x4806c000 {
> + compatible = "ti,omap4-uart";
> + ti,hwmods = "uart2";
> + clock-frequency = <4800>;
> + };
> +
> + uart3: serial@0x4802 {
> + compatible = "ti,omap4-uart";
> + ti,hwmods = "uart3";
> + clock-frequency = <4800>;
> + };
> +
> + uart4: serial@0x4806e000 {
> + compatible = "ti,omap4-uart";
> + ti,hwmods = "uart4";
> + clock-frequency = <4800>;
> + };
>   };
>  };
> diff --git a/arch/arm/mach-omap2/board-generic.c 
> b/arch/arm/mach-omap2/board-generic.c
> index fb55fa3..bb72809 100644
> --- a/arch/arm/mach-omap2/board-generic.c
> +++ b/arch/arm/mach-omap2/board-generic.c
> @@ -70,7 +70,6 @@ static void __init omap_generic_init(void)
>   if (node)
>   irq_domain_add_simple(node, 0);
>  
> - omap_serial_init();
>   omap_sdrc_init(NULL, NULL);
>  
>   of_platform_populate(NULL, omap_dt_match_table, NULL, NULL);


___
linaro-dev mailing list
linaro-dev@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev


Re: [PATCH v2 3/4] omap-serial: Add minimal device tree support

2011-11-28 Thread Rob Herring
On 11/22/2011 07:44 AM, Rajendra Nayak wrote:
> Adapt the driver to device tree and pass minimal platform
> data from device tree needed for console boot.
> No power management features will be suppported for now
> since it requires more tweaks around OCP settings
> to toggle forceidle/noidle/smaridle bits and handling

typo: smartidle

> remote wakeup and dynamic muxing.
> 
> Signed-off-by: Rajendra Nayak 

Acked-by: Rob Herring 

Rob

> ---
>  .../devicetree/bindings/serial/omap_serial.txt |   10 
>  drivers/tty/serial/omap-serial.c   |   45 ++-
>  2 files changed, 52 insertions(+), 3 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/serial/omap_serial.txt
> 
> diff --git a/Documentation/devicetree/bindings/serial/omap_serial.txt 
> b/Documentation/devicetree/bindings/serial/omap_serial.txt
> new file mode 100644
> index 000..342eedd
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/serial/omap_serial.txt
> @@ -0,0 +1,10 @@
> +OMAP UART controller
> +
> +Required properties:
> +- compatible : should be "ti,omap2-uart" for OMAP2 controllers
> +- compatible : should be "ti,omap3-uart" for OMAP3 controllers
> +- compatible : should be "ti,omap4-uart" for OMAP4 controllers
> +- ti,hwmods : Must be "uart", n being the instance number (1-based)
> +
> +Optional properties:
> +- clock-frequency : frequency of the clock input to the UART
> diff --git a/drivers/tty/serial/omap-serial.c 
> b/drivers/tty/serial/omap-serial.c
> index f14b9c5..5aa524e 100644
> --- a/drivers/tty/serial/omap-serial.c
> +++ b/drivers/tty/serial/omap-serial.c
> @@ -38,6 +38,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  
>  #include 
>  #include 
> @@ -1324,6 +1325,19 @@ static void uart_tx_dma_callback(int lch, u16 
> ch_status, void *data)
>   return;
>  }
>  
> +static struct omap_uart_port_info *of_get_uart_port_info(struct device *dev)
> +{
> + struct omap_uart_port_info *omap_up_info;
> +
> + omap_up_info = devm_kzalloc(dev, sizeof(*omap_up_info), GFP_KERNEL);
> + if (!omap_up_info)
> + return NULL; /* out of memory */
> +
> + of_property_read_u32(dev->of_node, "clock-frequency",
> +  &omap_up_info->uartclk);
> + return omap_up_info;
> +}
> +
>  static int serial_omap_probe(struct platform_device *pdev)
>  {
>   struct uart_omap_port   *up;
> @@ -1331,6 +1345,9 @@ static int serial_omap_probe(struct platform_device 
> *pdev)
>   struct omap_uart_port_info *omap_up_info = pdev->dev.platform_data;
>   int ret = -ENOSPC;
>  
> + if (pdev->dev.of_node)
> + omap_up_info = of_get_uart_port_info(&pdev->dev);
> +
>   mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>   if (!mem) {
>   dev_err(&pdev->dev, "no mem resource?\n");
> @@ -1375,9 +1392,20 @@ static int serial_omap_probe(struct platform_device 
> *pdev)
>   up->port.regshift = 2;
>   up->port.fifosize = 64;
>   up->port.ops = &serial_omap_pops;
> - up->port.line = pdev->id;
> - sprintf(up->name, "OMAP UART%d", up->port.line);
>  
> + if (pdev->dev.of_node)
> + up->port.line = of_alias_get_id(pdev->dev.of_node, "serial");
> + else
> + up->port.line = pdev->id;
> +
> + if (up->port.line < 0) {
> + dev_err(&pdev->dev, "failed to get alias/pdev id, errno %d\n",
> + up->port.line);
> + ret = -ENODEV;
> + goto err;
> + }
> +
> + sprintf(up->name, "OMAP UART%d", up->port.line);
>   up->port.mapbase = mem->start;
>   up->port.membase = ioremap(mem->start, resource_size(mem));
>   if (!up->port.membase) {
> @@ -1530,7 +1558,7 @@ static int serial_omap_runtime_suspend(struct device 
> *dev)
>   if (!up)
>   return -EINVAL;
>  
> - if (!pdata->enable_wakeup)
> + if (!pdata || !pdata->enable_wakeup)
>   return 0;
>  
>   if (pdata->get_context_loss_count)
> @@ -1591,12 +1619,23 @@ static const struct dev_pm_ops serial_omap_dev_pm_ops 
> = {
>   serial_omap_runtime_resume, NULL)
>  };
>  
> +#if defined(CONFIG_OF)
> +static const struct of_device_id omap_serial_of_match[] = {
> + { .compatible = "ti,omap2-uart" },
> + { .compatible = "ti,omap3-uart&quo

Re: [PATCH 3/3] ARM: omap: pass minimal SoC/board data for UART from dt

2011-11-16 Thread Rob Herring
On 11/16/2011 05:02 AM, Rajendra Nayak wrote:
> Pass minimal data needed for console boot, from dt, for
> OMAP4 panda/sdp and OMAP3 beagle boards, and get rid of the
> static initialization from generic board file.
> 
> Signed-off-by: Rajendra Nayak 
> ---
>  arch/arm/boot/dts/omap3-beagle.dts  |   17 +
>  arch/arm/boot/dts/omap3.dtsi|   27 +++
>  arch/arm/boot/dts/omap4-panda.dts   |   17 +
>  arch/arm/boot/dts/omap4-sdp.dts |   17 +
>  arch/arm/boot/dts/omap4.dtsi|   24 
>  arch/arm/mach-omap2/board-generic.c |1 -
>  6 files changed, 102 insertions(+), 1 deletions(-)
> 
> diff --git a/arch/arm/boot/dts/omap3-beagle.dts 
> b/arch/arm/boot/dts/omap3-beagle.dts
> index 9486be6..4c8f11e 100644
> --- a/arch/arm/boot/dts/omap3-beagle.dts
> +++ b/arch/arm/boot/dts/omap3-beagle.dts
> @@ -27,3 +27,20 @@
>   reg = <0x8000 0x2000>; /* 512 MB */
>   };
>  };
> +
> +&uart1 {
> + clock-frequency = <4800>;
> +};
> +
> +&uart2 {
> + clock-frequency = <4800>;
> +};
> +
> +&uart3 {
> + ti,console_hwmod;
> + clock-frequency = <4800>;
> +};
> +
> +&uart4 {
> + clock-frequency = <4800>;
> +};
> diff --git a/arch/arm/boot/dts/omap3.dtsi b/arch/arm/boot/dts/omap3.dtsi
> index d202bb5..ea591c5 100644
> --- a/arch/arm/boot/dts/omap3.dtsi
> +++ b/arch/arm/boot/dts/omap3.dtsi
> @@ -13,6 +13,13 @@
>  / {
>   compatible = "ti,omap3430", "ti,omap3";
>  
> + aliases {
> + uart1 = &uart1;
> + uart2 = &uart2;
> + uart3 = &uart3;
> + uart4 = &uart4;
> + };
> +
>   cpus {
>   cpu@0 {
>   compatible = "arm,cortex-a8";
> @@ -59,5 +66,25 @@
>   interrupt-controller;
>   #interrupt-cells = <1>;
>   };
> +
> + uart1: uart@1 {

Use the generic name serial and the address: uart1: serial@1234abcd

> + compatible = "ti,omap-uart";
> + ti,hwmods = "uart1";
> + };
> +
> + uart2: uart@2 {
> + compatible = "ti,omap-uart";
> + ti,hwmods = "uart2";
> + };
> +
> + uart3: uart@3 {
> + compatible = "ti,omap-uart";
> + ti,hwmods = "uart3";
> + };
> +
> + uart4: uart@4 {
> + compatible = "ti,omap-uart";
> + ti,hwmods = "uart4";
> + };
>   };
>  };
> diff --git a/arch/arm/boot/dts/omap4-panda.dts 
> b/arch/arm/boot/dts/omap4-panda.dts
> index c702657..aa65449 100644
> --- a/arch/arm/boot/dts/omap4-panda.dts
> +++ b/arch/arm/boot/dts/omap4-panda.dts
> @@ -27,3 +27,20 @@
>   reg = <0x8000 0x4000>; /* 1 GB */
>   };
>  };
> +
> +&uart1 {
> + clock-frequency = <4800>;
> +};
> +
> +&uart2 {
> + clock-frequency = <4800>;
> +};
> +
> +&uart3 {
> + ti,console_hwmod;
> + clock-frequency = <4800>;
> +};
> +
> +&uart4 {
> + clock-frequency = <4800>;
> +};
> diff --git a/arch/arm/boot/dts/omap4-sdp.dts b/arch/arm/boot/dts/omap4-sdp.dts
> index 066e28c..524f5bf 100644
> --- a/arch/arm/boot/dts/omap4-sdp.dts
> +++ b/arch/arm/boot/dts/omap4-sdp.dts
> @@ -27,3 +27,20 @@
>   reg = <0x8000 0x4000>; /* 1 GB */
>   };
>  };
> +
> +&uart1 {
> + clock-frequency = <4800>;
> +};
> +
> +&uart2 {
> + clock-frequency = <4800>;
> +};
> +
> +&uart3 {
> + ti,console_hwmod;
> + clock-frequency = <4800>;
> +};
> +
> +&uart4 {
> + clock-frequency = <4800>;

It doesn't seem that this frequency ever varies and is likely to be
replaced with clock bindings, so maybe just put it in the dtsi files.

Rob


___
linaro-dev mailing list
linaro-dev@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev


Re: [PATCH 1/3] ARM: omap_device: handle first time activation of console device

2011-11-16 Thread Rob Herring
Benoit,

On 11/16/2011 09:14 AM, Cousson, Benoit wrote:
> Hi Rob,
> 
> On 11/16/2011 3:50 PM, Rob Herring wrote:
>> On 11/16/2011 05:02 AM, Rajendra Nayak wrote:
>>> console device on OMAP is never reset or idled by hwmod post
>>> initial setup, early during boot, for obvious reasons not to
>>> break early debug prints thrown on console.
>>> This leaves the console device enabled at boot and the first activation
>>> of it using hwmod needs to be handled in such a way that a disable is
>>> called followed by an enable of the hwmod, the subsequent activations
>>> can then use the default activation technique.
>>>
>>> To handle this add a new binding to identify a hwmod which is used as
>>> the console device.
>>>
>>> This patch is based on the what is done in serial.c for non-DT builds.
>>>
>>> Signed-off-by: Rajendra Nayak
>>> ---
>>>   .../devicetree/bindings/arm/omap/omap.txt  |1 +
>>>   arch/arm/plat-omap/omap_device.c   |   33
>>> +++-
>>>   2 files changed, 33 insertions(+), 1 deletions(-)
>>>
>>> diff --git a/Documentation/devicetree/bindings/arm/omap/omap.txt
>>> b/Documentation/devicetree/bindings/arm/omap/omap.txt
>>> index dbdab40..46ffd41 100644
>>> --- a/Documentation/devicetree/bindings/arm/omap/omap.txt
>>> +++ b/Documentation/devicetree/bindings/arm/omap/omap.txt
>>> @@ -21,6 +21,7 @@ Required properties:
>>>   Optional properties:
>>>   - ti,no_idle_on_suspend: When present, it prevents the PM to idle
>>> the module
>>> during suspend.
>>> +- ti,console_hwmod: boolean, identifies the hwmod used as console
>>> device
>>>
>>
>> This doesn't seem right. Which console is not a h/w property. Why can't
>> you use aliases like other platforms are doing?
>>
>> Also, it's not clear in the documentation where this (and
>> ti,no_idle_on_suspend) should go in the DT. Both seem like they should
>> be kernel cmdline params.
> 
> That raise the question about using DT to pass linux specific parameter.
> We already discuss that on the mailing list, but never get a clear answer.
> It seems that DT is already used to provide some OS specific information
> using the "linux," prefix for example.

True, but "linux," properties will always face resistance and scrutiny.

> There are clearly a couple of parameters that can be provided by the
> bootloader, but that does not reflect a HW property.
> 
> So what is the guideline for that, should we stick to cmdline params for
> that?
> 

I would say that if the setting does not depend on something in the DT,
then the cmdline is the right place. If you have a property that
references a phandle, then it can't be on the cmdline. For console
selection, there is already a defined way to select it with
console=blah. And there is an agreed way to define indexes in the DT
with aliases.

How were these properties set without DT?

> Quoting Grant's documentation, runtime configuration is supported:
> 
> "Runtime configuration
> 
> In most cases, a DT will be the sole method of communicating data from
> firmware to the kernel, so also gets used to pass in runtime and
> configuration data like the kernel parameters string and the location of
> an initrd image.
> 
> Most of this data is contained in the /chosen node, and when booting
> Linux it will look something like this:
> 
> chosen {
> bootargs = "console=ttyS0,115200 loglevel=8";
> initrd-start = <0xc800>;
> initrd-end = <0xc820>;
> };
> 
> The bootargs property contains the kernel arguments, and the initrd-*
> properties define the address and size of an initrd blob. The chosen
> node may also optionally contain an arbitrary number of additional
> properties for platform-specific configuration data."
> 
> 
> Does that mean that this is supported only if the bootloader does not
> support cmdline?

No. I think it means chosen can be extended. However, how many other
chosen properties are defined? Not very many. Clearly, it's not a place
for adding whatever random property you want. chosen is really the boot
interface between the bootloader and kernel as it is ATAG type data.

Rob

___
linaro-dev mailing list
linaro-dev@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev


Re: [PATCH 2/3] omap-serial: Add minimal device tree support

2011-11-16 Thread Rob Herring
On 11/16/2011 05:02 AM, Rajendra Nayak wrote:
> Adapt the driver to device tree and pass minimal platform
> data from device tree needed for console boot.
> No power management features will be suppported for now
> since it requires more tweaks around OCP settings
> to toggle forceidle/noidle/smaridle bits and handling
> remote wakeup and dynamic muxing.
> 
> Signed-off-by: Rajendra Nayak 
> ---
>  .../devicetree/bindings/serial/omap_serial.txt |9 +
>  drivers/tty/serial/omap-serial.c   |   37 
> +++-
>  2 files changed, 45 insertions(+), 1 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/serial/omap_serial.txt
> 
> diff --git a/Documentation/devicetree/bindings/serial/omap_serial.txt 
> b/Documentation/devicetree/bindings/serial/omap_serial.txt
> new file mode 100644
> index 000..bf6d631
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/serial/omap_serial.txt
> @@ -0,0 +1,9 @@
> +OMAP UART controller
> +
> +Required properties:
> +- compatible : should be "ti,omap-uart"

This seems too generic. There are no h/w differences in the uart since
the 1st OMAP1 device?

> +- ti,hwmods : Must be "uart", n being the instance number (1-based)
> +
> +Optional properties:
> +- clock-frequency : frequency of the clock input to the UART
> +- ti,console_hwmod : boolean, UART used as debug console
> diff --git a/drivers/tty/serial/omap-serial.c 
> b/drivers/tty/serial/omap-serial.c
> index e1c8527..e3419c6 100644
> --- a/drivers/tty/serial/omap-serial.c
> +++ b/drivers/tty/serial/omap-serial.c
> @@ -38,6 +38,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  
>  #include 
>  #include 
> @@ -1322,6 +1323,22 @@ static void uart_tx_dma_callback(int lch, u16 
> ch_status, void *data)
>   return;
>  }
>  
> +static struct omap_uart_port_info *of_get_uart_port_info(struct device *dev)
> +{
> + struct omap_uart_port_info *omap_up_info;
> +
> + omap_up_info = devm_kzalloc(dev, sizeof(*omap_up_info), GFP_KERNEL);
> + if (!omap_up_info)
> + return NULL; /* out of memory */
> +
> + of_property_read_u32(dev->of_node, "clock-frequency",
> +  &(*omap_up_info).uartclk);

&omap_up_info->uartclk

You want 0 to be the default freq?

> +
> + return omap_up_info;
> +}
> +
> +static atomic_t omap_uart_next_id = ATOMIC_INIT(0);
> +
>  static int serial_omap_probe(struct platform_device *pdev)
>  {
>   struct uart_omap_port   *up;
> @@ -1329,6 +1346,11 @@ static int serial_omap_probe(struct platform_device 
> *pdev)
>   struct omap_uart_port_info *omap_up_info = pdev->dev.platform_data;
>   int ret = -ENOSPC;
>  
> + if (pdev->dev.of_node) {
> + omap_up_info = of_get_uart_port_info(&pdev->dev);
> + pdev->id = atomic_inc_return(&omap_uart_next_id) - 1;

I don't think a driver changing this value is a good idea. Look at other
serial drivers like iMX for how they use aliases.

> + }
> +
>   mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>   if (!mem) {
>   dev_err(&pdev->dev, "no mem resource?\n");
> @@ -1523,7 +1545,7 @@ static int serial_omap_runtime_suspend(struct device 
> *dev)
>   if (!up)
>   return -EINVAL;
>  
> - if (!pdata->enable_wakeup)
> + if (!pdata || !pdata->enable_wakeup)
>   return 0;
>  
>   if (pdata->get_context_loss_count)
> @@ -1582,12 +1604,25 @@ static const struct dev_pm_ops serial_omap_dev_pm_ops 
> = {
>   serial_omap_runtime_resume, NULL)
>  };
>  
> +#if defined(CONFIG_OF)
> +static const struct of_device_id omap_serial_of_match[] = {
> + {
> + .compatible = "ti,omap-uart",
> + },
> + {},
> +};
> +MODULE_DEVICE_TABLE(of, omap_serial_of_match);
> +#else
> +#define omap_serial_of_match NULL
> +#endif
> +
>  static struct platform_driver serial_omap_driver = {
>   .probe  = serial_omap_probe,
>   .remove = serial_omap_remove,
>   .driver = {
>   .name   = DRIVER_NAME,
>   .pm = &serial_omap_dev_pm_ops,
> + .of_match_table = omap_serial_of_match,

Use of_match_ptr and get rid of the #else

Rob

___
linaro-dev mailing list
linaro-dev@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev


Re: [PATCH 1/3] ARM: omap_device: handle first time activation of console device

2011-11-16 Thread Rob Herring
On 11/16/2011 05:02 AM, Rajendra Nayak wrote:
> console device on OMAP is never reset or idled by hwmod post
> initial setup, early during boot, for obvious reasons not to
> break early debug prints thrown on console.
> This leaves the console device enabled at boot and the first activation
> of it using hwmod needs to be handled in such a way that a disable is
> called followed by an enable of the hwmod, the subsequent activations
> can then use the default activation technique.
> 
> To handle this add a new binding to identify a hwmod which is used as
> the console device.
> 
> This patch is based on the what is done in serial.c for non-DT builds.
> 
> Signed-off-by: Rajendra Nayak 
> ---
>  .../devicetree/bindings/arm/omap/omap.txt  |1 +
>  arch/arm/plat-omap/omap_device.c   |   33 
> +++-
>  2 files changed, 33 insertions(+), 1 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/arm/omap/omap.txt 
> b/Documentation/devicetree/bindings/arm/omap/omap.txt
> index dbdab40..46ffd41 100644
> --- a/Documentation/devicetree/bindings/arm/omap/omap.txt
> +++ b/Documentation/devicetree/bindings/arm/omap/omap.txt
> @@ -21,6 +21,7 @@ Required properties:
>  Optional properties:
>  - ti,no_idle_on_suspend: When present, it prevents the PM to idle the module
>during suspend.
> +- ti,console_hwmod: boolean, identifies the hwmod used as console device
>  

This doesn't seem right. Which console is not a h/w property. Why can't
you use aliases like other platforms are doing?

Also, it's not clear in the documentation where this (and
ti,no_idle_on_suspend) should go in the DT. Both seem like they should
be kernel cmdline params.

Rob

___
linaro-dev mailing list
linaro-dev@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev


Re: [PATCH] distclean: Remove generated .dtb files

2011-11-07 Thread Rob Herring
On 11/05/2011 01:19 AM, Dirk Behme wrote:
> The patch 'arm/dt: Add dtb make rule' adds support to
> create a .dtb file. But this is never removed afterwards.
> Remove the generated .dtb file if 'distclean' is called.
> 
> Signed-off-by: Dirk Behme 
> CC: Rob Herring 
> CC: Shawn Guo 
> CC: Jason Liu 
> CC: Grant Likely 
> ---

Acked-by: Rob Herring 

>  arch/arm/boot/Makefile |2 ++
>  1 files changed, 2 insertions(+), 0 deletions(-)
> 
> diff --git a/arch/arm/boot/Makefile b/arch/arm/boot/Makefile
> index 1cd9b0a..08f0d35 100644
> --- a/arch/arm/boot/Makefile
> +++ b/arch/arm/boot/Makefile
> @@ -71,6 +71,8 @@ $(obj)/%.dtb: $(src)/dts/%.dts
>  
>  $(obj)/dtbs: $(addprefix $(obj)/, $(dtb-y))
>  
> +clean-files := *.dtb
> +
>  quiet_cmd_uimage = UIMAGE  $@
>cmd_uimage = $(CONFIG_SHELL) $(MKIMAGE) -A arm -O linux -T kernel \
>  -C none -a $(LOADADDR) -e $(STARTADDR) \


___
linaro-dev mailing list
linaro-dev@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev


Re: sneak preview of kernel ci views

2011-10-14 Thread Rob Herring
On 10/12/2011 07:16 PM, Michael Hudson-Doyle wrote:
> Hi all,
> 
> I've done a stealth deploy (stealth as in "no links point to this page")
> of the first cut of the kernel ci view I've been working on:
> 
> http://validation.linaro.org/lava-server/kernel-ci-views/index
> 
> It's very much an alpha-style view at the moment -- in particular the
> computation of the new passes and failures is a bit wonky -- but I've
> been working away for a while on this and it seemed like time to
> actually get a live page out to the public and stop emailing screenshots
> to slightly randomly chosen people.
> 
> If you see problems, you can file bugs at:
> 
> https://bugs.launchpad.net/lava-kernel-ci-views
> 
> or make more general comments here.
> 

How do you get the build log of a failed build? It wasn't very obvious:

Click on the failed build
Click on Bundle viewer (What's a bundle was my first question)
Find a url that sounds useful:
https://ci.linaro.org/jenkins/job/linux-next_panda-omap2plus/340/consoleText
Paste that link into your browser.
Search thru it to find compile errors.

Rob

___
linaro-dev mailing list
linaro-dev@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev


Re: [PATCH v2 1/7] clk: Add a generic clock infrastructure

2011-10-03 Thread Rob Herring
Mike,

On 09/22/2011 05:26 PM, Mike Turquette wrote:

> diff --git a/drivers/clk/clkdev.c b/drivers/clk/clkdev.c
> index 6db161f..e2a9719 100644
> --- a/drivers/clk/clkdev.c
> +++ b/drivers/clk/clkdev.c
> @@ -23,6 +23,13 @@
>  static LIST_HEAD(clocks);
>  static DEFINE_MUTEX(clocks_mutex);
>  
> +/* For USE_COMMON_STRUCT_CLK, these are provided in clk.c, but not exported
> + * through other headers; we don't want them used anywhere but here. */
> +#ifdef CONFIG_USE_COMMON_STRUCT_CLK
> +extern int __clk_get(struct clk *clk);
> +extern void __clk_put(struct clk *clk);
> +#endif
> +

This is dead code left from prior versions.

Rob

___
linaro-dev mailing list
linaro-dev@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev


Re: [PATCH v2 1/7] clk: Add a generic clock infrastructure

2011-10-03 Thread Rob Herring
On 10/03/2011 09:25 AM, Mark Brown wrote:
> On Mon, Oct 03, 2011 at 09:17:30AM -0500, Rob Herring wrote:
>> On 09/22/2011 05:26 PM, Mike Turquette wrote:
> 
> A lot of stuff that should really have been cut plus...
> 
>>> +   if (clk->ops->get_parent)
>>> +   /* We don't to lock against prepare/enable here, as
>>> +* the clock is not yet accessible from anywhere */
>>> +   clk->parent = clk->ops->get_parent(clk->hw);
> 
>> I don't think this is going to work. This implies that the parent clock
>> is already registered. For simple clk trees, that's probably not an
>> issue, but for chips with lots of muxing it will be impossible to get
>> the order correct for all cases. This is not an issue today as most
>> clocks are statically created.
> 
>> I think what is needed is a 2 stage init. The 1st stage to create all
>> the clocks and a 2nd stage to build the tree once all clocks are created.
> 
>> Tracking the parents using struct clk_hw instead would help as long as
>> clocks are still statically allocated. However, that won't help for
>> devicetree.
> 
> This isn't in any way specific to clocks, right now the likely solution
> looks to be Grant's changes for retrying probe() as new devices come on
> line.  With that devices can return a code from their probe() which
> tells the driver core that they couldn't get all the resources they need
> and that it should retry the probe() if more devices come on-line.

Except SOC clocks are initialized very early before timers are up and
there can be a very high number of dependencies (every clock except
fixed clocks). With the driver probe retry, retrying is the exception,
not the rule.

Retrying would require every caller to maintain a list of clks to
retry. With 2 stages, you can move that into the core clock code.

There are not typically a large number of board-level/driver created
clocks, so ensuring correct register order is not really a problem. In
cases where there is a cross-driver dependency, the probe retry is a
good solution.

Rob

___
linaro-dev mailing list
linaro-dev@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev


Re: [PATCH v2 1/7] clk: Add a generic clock infrastructure

2011-10-03 Thread Rob Herring
On 09/22/2011 05:26 PM, Mike Turquette wrote:
> From: Jeremy Kerr 
> 
> We currently have ~21 definitions of struct clk in the ARM architecture,
> each defined on a per-platform basis. This makes it difficult to define
> platform- (or architecture-) independent clock sources without making
> assumptions about struct clk, and impossible to compile two
> platforms with different struct clks into a single image.
> 
> This change is an effort to unify struct clk where possible, by defining
> a common struct clk, and a set of clock operations. Different clock
> implementations can set their own operations, and have a standard
> interface for generic code. The callback interface is exposed to the
> kernel proper, while the clock implementations only need to be seen by
> the platform internals.
> 
> The interface is split into two halves:
> 
>  * struct clk, which is the generic-device-driver interface. This
>provides a set of functions which drivers may use to request
>enable/disable, query or manipulate in a hardware-independent manner.
> 
>  * struct clk_hw and struct clk_hw_ops, which is the hardware-specific
>interface. Clock drivers implement the ops, which allow the core
>clock code to implement the generic 'struct clk' API.
> 
> This allows us to share clock code among platforms, and makes it
> possible to dynamically create clock devices in platform-independent
> code.
> 
> Platforms can enable the generic struct clock through
> CONFIG_GENERIC_CLK. In this case, the clock infrastructure consists of a
> common, opaque struct clk, and a set of clock operations (defined per
> type of clock):
> 
>   struct clk_hw_ops {
>   int (*prepare)(struct clk_hw *);
>   void(*unprepare)(struct clk_hw *);
>   int (*enable)(struct clk_hw *);
>   void(*disable)(struct clk_hw *);
>   unsigned long   (*recalc_rate)(struct clk_hw *);
>   int (*set_rate)(struct clk_hw *,
>   unsigned long, unsigned long *);
>   long(*round_rate)(struct clk_hw *, unsigned long);
>   int (*set_parent)(struct clk_hw *, struct clk *);
>   struct clk *(*get_parent)(struct clk_hw *);
>   };
> 
> Platform clock code can register a clock through clk_register, passing a
> set of operations, and a pointer to hardware-specific data:
> 
>   struct clk_hw_foo {
>   struct clk_hw clk;
>   void __iomem *enable_reg;
>   };
> 
>   #define to_clk_foo(c) offsetof(c, clk_hw_foo, clk)
> 
>   static int clk_foo_enable(struct clk_hw *clk)
>   {
>   struct clk_foo *foo = to_clk_foo(clk);
>   raw_writeb(foo->enable_reg, 1);
>   return 0;
>   }
> 
>   struct clk_hw_ops clk_foo_ops = {
>   .enable = clk_foo_enable,
>   };
> 
> And in the platform initialisation code:
> 
>   struct clk_foo my_clk_foo;
> 
>   void init_clocks(void)
>   {
>   my_clk_foo.enable_reg = ioremap(...);
> 
>   clk_register(&clk_foo_ops, &my_clk_foo, NULL);
>   }
> 
> Changes from Thomas Gleixner .
> 
> The common clock definitions are based on a development patch from Ben
> Herrenschmidt .
> 
> TODO:
> 
>  * We don't keep any internal reference to the clock topology at present.
> 
> Signed-off-by: Jeremy Kerr 
> Signed-off-by: Thomas Gleixner 
> Signed-off-by: Mark Brown 
> Signed-off-by: Mike Turquette 
> ---
> Changes since v1:
> Create a dummy clk_unregister and prototype/document it and clk_register
> Constify struct clk_hw_ops
> Remove spinlock.h header, include kernel.h
> Use EOPNOTSUPP instead of ENOTSUPP
> Add might_sleep to clk_prepare/clk_unprepare stubs
> Properly init children hlist and child_node
> Whitespace and typo fixes
> 
>  drivers/clk/Kconfig  |3 +
>  drivers/clk/Makefile |1 +
>  drivers/clk/clk.c|  232 
> ++
>  drivers/clk/clkdev.c |7 ++
>  include/linux/clk.h  |  140 +++---
>  5 files changed, 371 insertions(+), 12 deletions(-)
>  create mode 100644 drivers/clk/clk.c
> 
> diff --git a/drivers/clk/Kconfig b/drivers/clk/Kconfig
> index 3530927..c53ed59 100644
> --- a/drivers/clk/Kconfig
> +++ b/drivers/clk/Kconfig
> @@ -5,3 +5,6 @@ config CLKDEV_LOOKUP
>  
>  config HAVE_MACH_CLKDEV
>   bool
> +
> +config GENERIC_CLK
> + bool
> diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile
> index 07613fa..570d5b9 100644
> --- a/drivers/clk/Makefile
> +++ b/drivers/clk/Makefile
> @@ -1,2 +1,3 @@
>  
>  obj-$(CONFIG_CLKDEV_LOOKUP)  += clkdev.o
> +obj-$(CONFIG_GENERIC_CLK)+= clk.o
> diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
> new file mode 100644
> index 000..1cd7315
> --- /dev/null
> +++ b/drivers/clk/clk.c
> @@ -0,0 +1,232 @@
> +/*
> + * Copyright (C) 2010-2011 Canonical Ltd 
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foun

Re: [PATCH v2 4/7] clk: Add simple gated clock

2011-09-26 Thread Rob Herring
On 09/26/2011 05:37 PM, Turquette, Mike wrote:
> On Mon, Sep 26, 2011 at 12:37 PM, Jamie Iles  wrote:
>> On Mon, Sep 26, 2011 at 02:10:32PM -0500, Rob Herring wrote:
>>> On 09/26/2011 01:40 PM, Jamie Iles wrote:
>>>> On Mon, Sep 26, 2011 at 01:33:08PM -0500, Rob Herring wrote:
>>>>>> +static void clk_gate_set_bit(struct clk_hw *clk)
>>>>>> +{
>>>>>> + struct clk_gate *gate = to_clk_gate(clk);
>>>>>> + u32 reg;
>>>>>> +
>>>>>> + reg = __raw_readl(gate->reg);
>>>>>> + reg |= BIT(gate->bit_idx);
>>>>>> + __raw_writel(reg, gate->reg);
>>>>>
>>>>> Don't these read-mod-writes need a spinlock around it?
>>>>>
>>>>> It's possible to have an enable bits and dividers in the same register.
>>>>> If you did a set_rate and while doing an enable/disable, there would be
>>>>> a problem. Also, it may be 2 different clocks in the same register, so
>>>>> the spinlock needs to be shared and not per clock.
>>>>
>>>> Well the prepare lock will be held here and I believe that would be
>>>> sufficient.
>>>
>>> No, the enable spinlock is protecting enable/disable. But set_rate is
>>> protected by the prepare mutex. So you clearly don't need locking if you
>>> have a register of only 1 bit enables. If you have a register accessed
>>> by both enable/disable and prepare/unprepare/set_rate, then you need
>>> some protection.
>>
>> OK fair point, but I would guess that if you had a clock like this then
>> you probably wouldn't use this simple gated clock would you?  (speaking
>> from my world where we have quite simple clocks ;-))
> 
> I think it is a safe assumption that if a register controls both
> enable/disable and some programmable divider then,
> 
> 1) those controls are probably for the same clock
> 2) that clock won't be using the cookie-cutter gated-clock
> implementation anyways

By definition of simple gated clock, the other bits have to be for
another clock. The restriction is that all the other bits can only be
clock gate bits.

> 
> Rob, do you feel these assumptions are OK and locking can remain the
> same in this patch?

Perhaps it is rare enough that it is not worth it use generic code in
this case. If so, the documentation should be clear about this
constraint. It is not something anyone will have hit before because
everyone used a single global lock. Now with the api being split between
2 locks, this adds a new complexity.

I think the simple gated clock code should be usable for any clock
controlled by a single bit in a 32-bit register independent of other
things in that register.

One example is MX1 in (mach-imx/clock-imx1.c). The CSCR register has
single bit enable for clk16m while hclk and clk48m have dividers in that
register.

Rob

___
linaro-dev mailing list
linaro-dev@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev


Re: [PATCH v2 4/7] clk: Add simple gated clock

2011-09-26 Thread Rob Herring
On 09/26/2011 01:40 PM, Jamie Iles wrote:
> Hi Rob,
> 
> On Mon, Sep 26, 2011 at 01:33:08PM -0500, Rob Herring wrote:
>> Mike,
>>
>> On 09/22/2011 05:26 PM, Mike Turquette wrote:
>>> From: Jeremy Kerr 
>>>
>>> Signed-off-by: Jeremy Kerr 
>>> Signed-off-by: Mark Brown 
>>> Signed-off-by: Jamie Iles 
>>> Signed-off-by: Mike Turquette 
>>> ---
>>> Changes since v1:
>>> Add copyright header
>>> Fold in Jamie's patch for set-to-disable clks
>>> Use BIT macro instead of shift
>>>
>>>  drivers/clk/Kconfig|4 ++
>>>  drivers/clk/Makefile   |1 +
>>>  drivers/clk/clk-gate.c |   78 
>>> 
>>>  include/linux/clk.h|   13 
>>>  4 files changed, 96 insertions(+), 0 deletions(-)
>>>  create mode 100644 drivers/clk/clk-gate.c
>>>
>>> diff --git a/drivers/clk/Kconfig b/drivers/clk/Kconfig
>>> index d8313d7..a78967c 100644
>>> --- a/drivers/clk/Kconfig
>>> +++ b/drivers/clk/Kconfig
>>> @@ -12,3 +12,7 @@ config GENERIC_CLK
>>>  config GENERIC_CLK_FIXED
>>> bool
>>> depends on GENERIC_CLK
>>> +
>>> +config GENERIC_CLK_GATE
>>> +   bool
>>> +   depends on GENERIC_CLK
>>> diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile
>>> index 9a3325a..d186446 100644
>>> --- a/drivers/clk/Makefile
>>> +++ b/drivers/clk/Makefile
>>> @@ -2,3 +2,4 @@
>>>  obj-$(CONFIG_CLKDEV_LOOKUP)+= clkdev.o
>>>  obj-$(CONFIG_GENERIC_CLK)  += clk.o
>>>  obj-$(CONFIG_GENERIC_CLK_FIXED)+= clk-fixed.o
>>> +obj-$(CONFIG_GENERIC_CLK_GATE) += clk-gate.o
>>> diff --git a/drivers/clk/clk-gate.c b/drivers/clk/clk-gate.c
>>> new file mode 100644
>>> index 000..a1d8e79
>>> --- /dev/null
>>> +++ b/drivers/clk/clk-gate.c
>>> @@ -0,0 +1,78 @@
>>> +/*
>>> + * Copyright (C) 2010-2011 Canonical Ltd 
>>> + *
>>> + * This program is free software; you can redistribute it and/or modify
>>> + * it under the terms of the GNU General Public License version 2 as
>>> + * published by the Free Software Foundation.
>>> + *
>>> + * Simple clk gate implementation
>>> + */
>>> +
>>> +#include 
>>> +#include 
>>> +#include 
>>
>> use linux/io.h
>>
>>> +
>>> +#define to_clk_gate(clk) container_of(clk, struct clk_gate, hw)
>>> +
>>> +static unsigned long clk_gate_get_rate(struct clk_hw *clk)
>>> +{
>>> +   return clk_get_rate(clk_get_parent(clk->clk));
>>> +}
>>> +
>>> +static void clk_gate_set_bit(struct clk_hw *clk)
>>> +{
>>> +   struct clk_gate *gate = to_clk_gate(clk);
>>> +   u32 reg;
>>> +
>>> +   reg = __raw_readl(gate->reg);
>>> +   reg |= BIT(gate->bit_idx);
>>> +   __raw_writel(reg, gate->reg);
>>
>> Don't these read-mod-writes need a spinlock around it?
>>
>> It's possible to have an enable bits and dividers in the same register.
>> If you did a set_rate and while doing an enable/disable, there would be
>> a problem. Also, it may be 2 different clocks in the same register, so
>> the spinlock needs to be shared and not per clock.
> 
> Well the prepare lock will be held here and I believe that would be 
> sufficient.

No, the enable spinlock is protecting enable/disable. But set_rate is
protected by the prepare mutex. So you clearly don't need locking if you
have a register of only 1 bit enables. If you have a register accessed
by both enable/disable and prepare/unprepare/set_rate, then you need
some protection.


> 
>>> +}
>>> +
>>> +static void clk_gate_clear_bit(struct clk_hw *clk)
>>> +{
>>> +   struct clk_gate *gate = to_clk_gate(clk);
>>> +   u32 reg;
>>> +
>>> +   reg = __raw_readl(gate->reg);
>>> +   reg &= ~BIT(gate->bit_idx);
>>> +   __raw_writel(reg, gate->reg);
>>> +}
>>> +
>>> +static int clk_gate_enable_set(struct clk_hw *clk)
>>> +{
>>> +   clk_gate_set_bit(clk);
>>> +
>>> +   return 0;
>>> +}
>>> +
>>> +static void clk_gate_disable_clear(struct clk_hw *clk)
>>> +{
>>> +   clk_gate_clear_bit(clk);
>>> +}
>>> +
>>> +struct clk_hw_ops clk_gate_set_enable_ops = {
>>
>> const?
> 
> Yup.
> 
>>> +   .recalc_rate = clk_gate_get_rate,
>>> +   .enable = clk_gate_enable_set,
>>> +   .disable = clk_gate_disable_clear,
>>> +};
>>> +EXPORT_SYMBOL_GPL(clk_gate_set_enable_ops);
>>> +
>>> +static int clk_gate_enable_clear(struct clk_hw *clk)
>>> +{
>>> +   clk_gate_clear_bit(clk);
>>> +
>>> +   return 0;
>>> +}
>>> +
>>> +static void clk_gate_disable_set(struct clk_hw *clk)
>>> +{
>>> +   clk_gate_set_bit(clk);
>>> +}
>>
>> Are these wrapper functions really needed? Just assign set_bit and
>> clear_bit functions directly to the ops structs. Only the ops struct
>> name is exposed to the user.
> 
> I used the wrappers because the .enable method has to return an int, but 
> the disable needs to return void.  It's either that or open code the 
> set/clear in each.

Okay. I missed that detail...

Rob

___
linaro-dev mailing list
linaro-dev@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev


Re: [PATCH v2 4/7] clk: Add simple gated clock

2011-09-26 Thread Rob Herring
Mike,

On 09/22/2011 05:26 PM, Mike Turquette wrote:
> From: Jeremy Kerr 
> 
> Signed-off-by: Jeremy Kerr 
> Signed-off-by: Mark Brown 
> Signed-off-by: Jamie Iles 
> Signed-off-by: Mike Turquette 
> ---
> Changes since v1:
> Add copyright header
> Fold in Jamie's patch for set-to-disable clks
> Use BIT macro instead of shift
> 
>  drivers/clk/Kconfig|4 ++
>  drivers/clk/Makefile   |1 +
>  drivers/clk/clk-gate.c |   78 
> 
>  include/linux/clk.h|   13 
>  4 files changed, 96 insertions(+), 0 deletions(-)
>  create mode 100644 drivers/clk/clk-gate.c
> 
> diff --git a/drivers/clk/Kconfig b/drivers/clk/Kconfig
> index d8313d7..a78967c 100644
> --- a/drivers/clk/Kconfig
> +++ b/drivers/clk/Kconfig
> @@ -12,3 +12,7 @@ config GENERIC_CLK
>  config GENERIC_CLK_FIXED
>   bool
>   depends on GENERIC_CLK
> +
> +config GENERIC_CLK_GATE
> + bool
> + depends on GENERIC_CLK
> diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile
> index 9a3325a..d186446 100644
> --- a/drivers/clk/Makefile
> +++ b/drivers/clk/Makefile
> @@ -2,3 +2,4 @@
>  obj-$(CONFIG_CLKDEV_LOOKUP)  += clkdev.o
>  obj-$(CONFIG_GENERIC_CLK)+= clk.o
>  obj-$(CONFIG_GENERIC_CLK_FIXED)  += clk-fixed.o
> +obj-$(CONFIG_GENERIC_CLK_GATE)   += clk-gate.o
> diff --git a/drivers/clk/clk-gate.c b/drivers/clk/clk-gate.c
> new file mode 100644
> index 000..a1d8e79
> --- /dev/null
> +++ b/drivers/clk/clk-gate.c
> @@ -0,0 +1,78 @@
> +/*
> + * Copyright (C) 2010-2011 Canonical Ltd 
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + * Simple clk gate implementation
> + */
> +
> +#include 
> +#include 
> +#include 

use linux/io.h

> +
> +#define to_clk_gate(clk) container_of(clk, struct clk_gate, hw)
> +
> +static unsigned long clk_gate_get_rate(struct clk_hw *clk)
> +{
> + return clk_get_rate(clk_get_parent(clk->clk));
> +}
> +
> +static void clk_gate_set_bit(struct clk_hw *clk)
> +{
> + struct clk_gate *gate = to_clk_gate(clk);
> + u32 reg;
> +
> + reg = __raw_readl(gate->reg);
> + reg |= BIT(gate->bit_idx);
> + __raw_writel(reg, gate->reg);

Don't these read-mod-writes need a spinlock around it?

It's possible to have an enable bits and dividers in the same register.
If you did a set_rate and while doing an enable/disable, there would be
a problem. Also, it may be 2 different clocks in the same register, so
the spinlock needs to be shared and not per clock.

> +}
> +
> +static void clk_gate_clear_bit(struct clk_hw *clk)
> +{
> + struct clk_gate *gate = to_clk_gate(clk);
> + u32 reg;
> +
> + reg = __raw_readl(gate->reg);
> + reg &= ~BIT(gate->bit_idx);
> + __raw_writel(reg, gate->reg);
> +}
> +
> +static int clk_gate_enable_set(struct clk_hw *clk)
> +{
> + clk_gate_set_bit(clk);
> +
> + return 0;
> +}
> +
> +static void clk_gate_disable_clear(struct clk_hw *clk)
> +{
> + clk_gate_clear_bit(clk);
> +}
> +
> +struct clk_hw_ops clk_gate_set_enable_ops = {

const?

> + .recalc_rate = clk_gate_get_rate,
> + .enable = clk_gate_enable_set,
> + .disable = clk_gate_disable_clear,
> +};
> +EXPORT_SYMBOL_GPL(clk_gate_set_enable_ops);
> +
> +static int clk_gate_enable_clear(struct clk_hw *clk)
> +{
> + clk_gate_clear_bit(clk);
> +
> + return 0;
> +}
> +
> +static void clk_gate_disable_set(struct clk_hw *clk)
> +{
> + clk_gate_set_bit(clk);
> +}

Are these wrapper functions really needed? Just assign set_bit and
clear_bit functions directly to the ops structs. Only the ops struct
name is exposed to the user.

> +
> +struct clk_hw_ops clk_gate_set_disable_ops = {

const?

Rob


___
linaro-dev mailing list
linaro-dev@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev


Re: Single zImage at Linaro Connect

2011-08-05 Thread Rob Herring
Deepak, Nicolas,

On 07/27/2011 09:58 PM, Nicolas Pitre wrote:
> 
> To everyone, and especially to those who are expected to work on this 
> topic next week, please find below a list of tasks that needs to be 
> investigated and/or accomplished.  I'll coordinate the work and collect 
> patches for the team.
> 
> If you have comments on this, or if you know about some omissions, 
> please feel free to provide them as a reply to this message.
> 
> I'd like to know if people are particularly interested in one (or more :-)) 
> items they would like to work on.  If so please say so as well.

Are you going to publish a summary of the week? For example, are there
any refinements to this list in terms of additional items or approach on
how to fix. Who is working each item and which ones need help?

Rob

> 
> Without further ado, here it is:
> 
> <><><><><>
> 
> 0) The so called "single zImage" project
> 
> We wish to provide the ability to build as many ARM platforms as 
> possible into a single kernel binary image. This will greatly simplify 
> the archive packaging and maintenance effort by having only one kernel 
> that could be built and booted on multiple ARM targets.  A side effect 
> of this is also to enforce better source code architecture even if the 
> resulting binaries are not always supporting multiple targets.
> 
> This work started a while ago.  Some initial description can be found 
> here:
> 
> https://wiki.ubuntu.com/Specs/ARMSingleKernel
> 
> Part of it has been implemented already, namely the runtime determined 
> PHYS_OFFSET, the AUTO_ZRELADDR and some other items referenced below.  
> But there is still a large amount of work remaining.
> 
> 1) Removal of any dependencies on  from generic header files
> 
> To see the current culprits:
> 
> $ git grep "#include " arch/arm/include/
> arch/arm/include/asm/clkdev.h:#include 
> arch/arm/include/asm/dma.h:#include 
> arch/arm/include/asm/floppy.h:#include 
> arch/arm/include/asm/gpio.h:#include 
> arch/arm/include/asm/hardware/dec21285.h:#include 
> arch/arm/include/asm/hardware/iop3xx-adma.h:#include 
> arch/arm/include/asm/hardware/iop3xx-gpio.h:#include 
> arch/arm/include/asm/hardware/sa.h:#include 
> arch/arm/include/asm/io.h:#include 
> arch/arm/include/asm/irq.h:#include 
> arch/arm/include/asm/mc146818rtc.h:#include 
> arch/arm/include/asm/memory.h:#include 
> arch/arm/include/asm/mtd-xip.h:#include 
> arch/arm/include/asm/pci.h:#include  /* for PCIBIOS_MIN_* */
> arch/arm/include/asm/pgtable.h:#include 
> arch/arm/include/asm/system.h:#include 
> arch/arm/include/asm/timex.h:#include 
> arch/arm/include/asm/vga.h:#include 
> 
> 1.1) mach/memory.h
> 
> This may contain the following defines:
> 
> 1.1.1) ARM_DMA_ZONE_SIZE
> 
> This can be eliminated by moving that value into struct machine_desc.
> The work is done already, but presented as an example for other tasks: 
> http://git.linaro.org/gitweb?p=people/nico/linux.git;a=shortlog;h=refs/heads/dma
> And as of now this is merged in mainline already for v3.1-rc1.
> 
> 1.1.2) PLAT_PHYS_OFFSET
> 
> Most occurrences can be eliminated.  With CONFIG_ARM_PATCH_PHYS_VIRT, it 
> is possible to determine PHYS_OFFSET at run time.  Remains to remove the 
> direct uses, mostly by mdesc->boot_params initializers.  Changing 
> boot_params into atag_offset has two effects: that makes it clearer that 
> it is only about ATAGs and not DT, and a relative offset plays more 
> nicely with a runtime determined PHYS_OFFSET.
> 
> This work is done but not yet accepted:
> http://news.gmane.org/group/gmane.linux.ports.arm.kernel/thread=123480
> 
> 1.1.3) FLUSH_BASE, FLUSH_BASE_PHYS, FLUSH_BASE_MINICACHE, UNCACHEABLE_ADDR
> 
> Those are StrongARM related constants, and different for each variants.
> Fixing this involves making the virtual addresses constant for all 
> variants, and hiding the differences in the physical addresses during 
> the actual mapping.
> 
> The solution is here:
> http://news.gmane.org/group/gmane.linux.ports.arm.kernel/thread=123477/force_load=t/focus=124849
> 
> 1.1.4) CONSISTENT_DMA_SIZE
> 
> Maybe the CMA work will make this obsolete and the consistent DMA area 
> could be dynamically adjusted.  In the mean time, the easiest solution 
> is probably to store this in the machine_desc structure just like with 
> ARM_DMA_ZONE_SIZE.
> 
> This has not been addressed yet.
> 
> 1.1.5) Other weird things
> 
> Some machines have non linear memory maps or bus address translations, 
> sparsemem, etc. Examples of that are:
> 
> arch/arm/mach-realview/include/mach/memory.h
> arch/arm/mach-integrator/include/mach/memory.h
> 
> I think solving this is out of scope for this round.  Deleting 
> arch/arm/mach-*/include/mach/memory.h can't be done universally.  So a 
> new Kconfig symbol (NO_MACH_MEMORY_H) is introduced to indicate which 
> machine class has its legacy  file removed.  The single 
> zImage for multiple targets will be restricted, amongst other things, to 
> those machines or SOCs with tha

Re: Single zImage at Linaro Connect

2011-07-29 Thread Rob Herring
On 07/29/2011 07:40 AM, Nicolas Pitre wrote:
> On Fri, 29 Jul 2011, Dave Martin wrote:
> 
>> On Thu, Jul 28, 2011 at 02:44:17PM -0400, Nicolas Pitre wrote:
>>> On Thu, 28 Jul 2011, Dave Martin wrote:
>>
 I have a slightly biased interest in this, since ARM seems to like
 funky memory maps for many of its newer boards, and it would be
 unfortunate for these to get left out of the whole single effort.

 Of course we could include those platforms in non-sparsemem kernels,
 but since that will often mean sacrificing half the RAM, this is
 far from ideal.
>>>
>>> Maybe that half could simply be pushed to home.
>>
>> Eh?
> 
> Euh...  I meant "himem".  No idea how my fingers turned that into 
> "home".
> 
 We could even embed the printch() function body into the DT if we
 wanted to make the kernel's job even simpler.  Realistic implementations
 of this function are tiny, so this shouldn't be too cumbersome.
 That really seems an abuse of the DT though, since this goes beyond
 just describing the hardware.
>>>
>>> Well... I'm not entirely against the idea.  This could be seen as the 
>>> most efficient way to describe how to output a character over some 
>>> serial device for the given hardware.  The danger is that some vendors 
>>> might be tempted to abuse that idea by stuffing BIOS-like services in 
>>> there that the kernel would have to cope with.
>>
>> See my reply to Arnd...
> 
> What might be done is to describe the data FIFO register location, and 
> the FIFO full bit mask, and the FIFO empty bit mask.  That's all we need 
> really.
> 
Except for RPC which outputs to video memory. We don't care about that
one for single kernel, but that may limit a new common solution.

BTW, I did implement a conversion to use debug macro code for
uncompress, but it doesn't work for RPC, OMAP and Davinci. So while it
shrinks the the code by over 2K lines, we probably need a new approach
like you suggest. The patches are here if you want to take a look:

git://git.jdl.com/software/linux-3.0.git
http://git.jdl.com/gitweb/?p=linux-3.0.git;a=shortlog;h=refs/heads/uncompress

Rob

___
linaro-dev mailing list
linaro-dev@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev


Re: Single zImage at Linaro Connect

2011-07-28 Thread Rob Herring
On 07/27/2011 09:58 PM, Nicolas Pitre wrote:
> 
> To everyone, and especially to those who are expected to work on this 
> topic next week, please find below a list of tasks that needs to be 
> investigated and/or accomplished.  I'll coordinate the work and collect 
> patches for the team.
> 
> If you have comments on this, or if you know about some omissions, 
> please feel free to provide them as a reply to this message.
> 
> I'd like to know if people are particularly interested in one (or more :-)) 
> items they would like to work on.  If so please say so as well.
> 
> Without further ado, here it is:
> 
> <><><><><>
> 
> 0) The so called "single zImage" project
> 
> We wish to provide the ability to build as many ARM platforms as 
> possible into a single kernel binary image. This will greatly simplify 
> the archive packaging and maintenance effort by having only one kernel 
> that could be built and booted on multiple ARM targets.  A side effect 
> of this is also to enforce better source code architecture even if the 
> resulting binaries are not always supporting multiple targets.
> 
> This work started a while ago.  Some initial description can be found 
> here:
> 
> https://wiki.ubuntu.com/Specs/ARMSingleKernel
> 
> Part of it has been implemented already, namely the runtime determined 
> PHYS_OFFSET, the AUTO_ZRELADDR and some other items referenced below.  
> But there is still a large amount of work remaining.
> 
> 1) Removal of any dependencies on  from generic header files
> 
> To see the current culprits:
> 
> $ git grep "#include " arch/arm/include/
> arch/arm/include/asm/clkdev.h:#include 
> arch/arm/include/asm/dma.h:#include 
> arch/arm/include/asm/floppy.h:#include 
> arch/arm/include/asm/gpio.h:#include 
> arch/arm/include/asm/hardware/dec21285.h:#include 
> arch/arm/include/asm/hardware/iop3xx-adma.h:#include 
> arch/arm/include/asm/hardware/iop3xx-gpio.h:#include 
> arch/arm/include/asm/hardware/sa.h:#include 
> arch/arm/include/asm/io.h:#include 
> arch/arm/include/asm/irq.h:#include 
> arch/arm/include/asm/mc146818rtc.h:#include 
> arch/arm/include/asm/memory.h:#include 
> arch/arm/include/asm/mtd-xip.h:#include 
> arch/arm/include/asm/pci.h:#include  /* for PCIBIOS_MIN_* */
> arch/arm/include/asm/pgtable.h:#include 
> arch/arm/include/asm/system.h:#include 
> arch/arm/include/asm/timex.h:#include 
> arch/arm/include/asm/vga.h:#include 
> 
> 1.1) mach/memory.h
> 
> This may contain the following defines:
> 
> 1.1.1) ARM_DMA_ZONE_SIZE
> 
> This can be eliminated by moving that value into struct machine_desc.
> The work is done already, but presented as an example for other tasks: 
> http://git.linaro.org/gitweb?p=people/nico/linux.git;a=shortlog;h=refs/heads/dma
> And as of now this is merged in mainline already for v3.1-rc1.
> 
> 1.1.2) PLAT_PHYS_OFFSET
> 
> Most occurrences can be eliminated.  With CONFIG_ARM_PATCH_PHYS_VIRT, it 
> is possible to determine PHYS_OFFSET at run time.  Remains to remove the 
> direct uses, mostly by mdesc->boot_params initializers.  Changing 
> boot_params into atag_offset has two effects: that makes it clearer that 
> it is only about ATAGs and not DT, and a relative offset plays more 
> nicely with a runtime determined PHYS_OFFSET.
> 
> This work is done but not yet accepted:
> http://news.gmane.org/group/gmane.linux.ports.arm.kernel/thread=123480
> 
> 1.1.3) FLUSH_BASE, FLUSH_BASE_PHYS, FLUSH_BASE_MINICACHE, UNCACHEABLE_ADDR
> 
> Those are StrongARM related constants, and different for each variants.
> Fixing this involves making the virtual addresses constant for all 
> variants, and hiding the differences in the physical addresses during 
> the actual mapping.
> 
> The solution is here:
> http://news.gmane.org/group/gmane.linux.ports.arm.kernel/thread=123477/force_load=t/focus=124849
> 
> 1.1.4) CONSISTENT_DMA_SIZE
> 
> Maybe the CMA work will make this obsolete and the consistent DMA area 
> could be dynamically adjusted.  In the mean time, the easiest solution 
> is probably to store this in the machine_desc structure just like with 
> ARM_DMA_ZONE_SIZE.
> 
> This has not been addressed yet.
> 
> 1.1.5) Other weird things
> 
> Some machines have non linear memory maps or bus address translations, 
> sparsemem, etc. Examples of that are:
> 
> arch/arm/mach-realview/include/mach/memory.h
> arch/arm/mach-integrator/include/mach/memory.h
> 
> I think solving this is out of scope for this round.  Deleting 
> arch/arm/mach-*/include/mach/memory.h can't be done universally.  So a 
> new Kconfig symbol (NO_MACH_MEMORY_H) is introduced to indicate which 
> machine class has its legacy  file removed.  The single 
> zImage for multiple targets will be restricted, amongst other things, to 
> those machines or SOCs with that symbol selected.  Partial result here:
> http://git.linaro.org/gitweb?p=people/nico/linux.git;a=shortlog;h=refs/heads/redux
> 
> 1.2) mach/io.h
> 
> This contains things like IO_SPACE_LIMIT, __io(), __mem_pci(), and 
> sometimes _

Re: [PATCH] ARM: do not mark CPU 0 as hotpluggable

2011-07-20 Thread Rob Herring
On 07/20/2011 06:32 PM, Mike Turquette wrote:
> A quick poll of the ARM platforms that implement CPU Hotplug support
> shows that every platform treats CPU 0 as a special case that cannot be
> hotplugged.  In fact every platform has identical code for
> platform_cpu_die which returns -EPERM in the case of CPU 0.
> 
> The user-facing sysfs interfaces should reflect this by not populating
> an 'online' entry for CPU 0 at all.  This better reflects reality by
> making it clear to users that CPU 0 cannot be hotplugged.
> 
> This patch prevents CPU 0 from being marked as hotpluggable on all ARM
> platforms during CPU registration.  This in turn prevents the creation
> of an 'online' sysfs interface for that CPU.
> 
Unless there is a kernel limitation why CPU0 can't be hot unplugged,
then this should remain a platform decision. This may be another case of
everybody just copying other platforms' code, not a platform limitation.

Rob

> Signed-off-by: Mike Turquette 
> ---
>  arch/arm/kernel/setup.c |3 ++-
>  1 files changed, 2 insertions(+), 1 deletions(-)
> 
> diff --git a/arch/arm/kernel/setup.c b/arch/arm/kernel/setup.c
> index ed11fb0..a5fc969 100644
> --- a/arch/arm/kernel/setup.c
> +++ b/arch/arm/kernel/setup.c
> @@ -940,7 +940,8 @@ static int __init topology_init(void)
>  
>   for_each_possible_cpu(cpu) {
>   struct cpuinfo_arm *cpuinfo = &per_cpu(cpu_data, cpu);
> - cpuinfo->cpu.hotpluggable = 1;
> + if (cpu)
> + cpuinfo->cpu.hotpluggable = 1;
>   register_cpu(&cpuinfo->cpu, cpu);
>   }
>  


___
linaro-dev mailing list
linaro-dev@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev


Re: Getting rid of alignment faults in userspace

2011-06-17 Thread Rob Herring
On 06/17/2011 08:11 AM, Andy Green wrote:
> On 06/17/2011 01:10 PM, Somebody in the thread at some point said:
> 
> Hi -
> 
>> I've recently become aware that a few packages are causing alignment
>> faults on ARM, and are relying on the alignment fixup emulation code in
>> the kernel in order to work.
> 
> Just a FYI a lot of later ARM chips are solving alignment fixups in
> hardware in the Bus Interface Unit, so the problems won't show up in
> kernel stats.
> 
>> Such faults are very expensive in terms of CPU cycles, and can generally
>> only result from wrong code (for example, C/C++ code which violates the
>> relevant language standards, assembler which makes invalid assumptions,
>> or functions called with misaligned pointers due to other bugs).
> 
> Agreed it's usually evidence of something broken and / or evil in the code.
> 
> There is still going to be a small cost even in hardware fixup so this
> is very much worth solving despite it's "becoming invisible" because the
> chips are hiding / solving it already.
> 

But I believe that h/w feature is turned off in Linux by default. You
have to add noalign to the kernel command line to enable.

Rob

___
linaro-dev mailing list
linaro-dev@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev


Re: [PATCH 00/11] RFC: DA9053 PMIC driver

2011-06-10 Thread Rob Herring

On 06/09/2011 10:50 PM, Ying-Chun Liu (PaulLiu) wrote:

From: "Ying-Chun Liu (PaulLiu)"

Hi. I'm trying to push DA9053 PMIC driver to mainline kernel.
Please help me to review these patches.
Many Thanks.

Ying-Chun Liu (PaulLiu) (11):
   PMIC: Add DA9053 headers from Dialog


Is this for the 9053 or...


   PMIC: Add GPIO Driver for Dialog DA9052


9052... Probably should be 905x as they are supposed to be similar.

Did the 9052 stuff ever go in?

Rob

___
linaro-dev mailing list
linaro-dev@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev


Re: Usefulness of GCC's 64bit __sync_* ops on ARM

2011-05-06 Thread Rob Herring

Ken,

On 05/06/2011 10:25 AM, Richard Earnshaw wrote:


On Fri, 2011-05-06 at 17:06 +0200, Ken Werner wrote:

Hi,

We've been thinking about adding support for the built-in functions for 64bit
atomic memory access and I'd like to know if this is of any interest.
Currently the main use of these functions seems to be to implement (SMP safe)
locking mechanisms where the existent 32bit memory ops are sufficient.
However, there might be code out there that implements a parallel algorithm
using 64bit atomic memory operations.


I've been told (though I've not verified it) that the Membase NoSQL
database needs 64-bit atomic operations to work effectively
(http://www.membase.org/downloadsrc)



By work effectively, that means they are needed to get membase to build 
and run. Without gcc support you have to copy the routines out of the 
kernel and link them with membase.


Rob

___
linaro-dev mailing list
linaro-dev@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev


Re: ION: google's answer to GEM?

2011-04-12 Thread Rob Herring

On 04/12/2011 11:12 AM, Clark, Rob wrote:

hmm,

https://review.source.android.com/#change,22239

 From a quick look, it appears to be doing a similar thing as GEM..
although as a stand-alone driver rather than coupled with display
driver (drm).


ION is also the name of Nvidia's Atom companion chipset.

Rob

___
linaro-dev mailing list
linaro-dev@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev


Re: [PATCH 5/7] mmc: support sdhci-esdhc-imx as an OF device

2011-03-17 Thread Rob Herring

On 03/17/2011 03:22 PM, Grant Likely wrote:

[cc'ing linux-mmc to continue this discussion]

On Wed, Mar 16, 2011 at 10:39:16PM +0800, Shawn Guo wrote:

On Tue, Mar 15, 2011 at 01:59:26PM -0600, Grant Likely wrote:

On Mon, Mar 14, 2011 at 10:25:57PM +0800, Shawn Guo wrote:

Signed-off-by: Shawn Guo


dt support can be added directly to sdchi-pltfm.c drivers now.  There
is no longer any need to use sdhci-of-core.c any more.  For an
example, see the patch titled "of/tegra: add sdhci device tree
handling" in my devicetree/test branch.


I mentioned this a little bit in the cover letter of the patch set
as below.

"This patch set is to support sdhci-esdhc-imx as an OF device.  As
there is already powerpc based esdhc OF support, it chose to add OF
support for imx esdhc driver in a different way from what sdhci-tegra
did."


I should read your descriptions more carefully.  :-)


The tegra approach you made was one of the two options I had, and I
happened to love the another more, as it consolidates the eSDHC OF
driver for Freescale MPCxxx and i.MX family.


Heh, I don't dispute the value of merging code.  However, with this
approach it means that DT and non-DT imx platforms will be using
different drivers for the same device.  Given the choices, I'd
rather see the imx driver used in both DT and non-DT situations
instead of sharing code with the powerpc version.  I've learnt the
hard way that it is just too painful having two drivers for the same
hardware; particularly when the only difference is the method used to
probe them.

Actually, what I'd *really* rather see is the powerpc code migrated
over to sdhci_pltfm.c, and then have the imx compatible value added to
it.  I'll make sure to get some help from the Freescale powerpc folks
to test any patch you produce to that end.


Based on past experience, there will be differences between imx and ppc 
h/w even though it is the "same" block.


Rob

___
linaro-dev mailing list
linaro-dev@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev


Re: [PATCH] arm/dt: Add dtb make rule

2011-03-14 Thread Rob Herring

Grant,

On 03/10/2011 01:46 PM, Rob Herring wrote:

From: Rob Herring

Add a make rule to compile dt blobs for ARM.

Signed-off-by: Rob Herring


Can you pick this one up in your ARM tree.

Rob

___
linaro-dev mailing list
linaro-dev@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev


Re: [PATCH 2/7] arm/dt: add pad configurations for mx51 babbage

2011-03-14 Thread Rob Herring

Shawn,

On 03/14/2011 09:25 AM, Shawn Guo wrote:

The pad configuration is something common between dt and non-dt
kernel, so it can be copied from non-dt code directly.

Signed-off-by: Shawn Guo
---
  arch/arm/mach-mx5/board-dt.c |   94 ++
  1 files changed, 94 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mach-mx5/board-dt.c b/arch/arm/mach-mx5/board-dt.c
index 45d1e37..4850251 100644
--- a/arch/arm/mach-mx5/board-dt.c
+++ b/arch/arm/mach-mx5/board-dt.c
@@ -31,6 +31,97 @@

  #include "devices.h"

+static iomux_v3_cfg_t mx51babbage_pads[] = {
+   /* UART1 */
+   MX51_PAD_UART1_RXD__UART1_RXD,
+   MX51_PAD_UART1_TXD__UART1_TXD,
+   MX51_PAD_UART1_RTS__UART1_RTS,
+   MX51_PAD_UART1_CTS__UART1_CTS,
+
+   /* UART2 */
+   MX51_PAD_UART2_RXD__UART2_RXD,
+   MX51_PAD_UART2_TXD__UART2_TXD,
+
+   /* UART3 */
+   MX51_PAD_EIM_D25__UART3_RXD,
+   MX51_PAD_EIM_D26__UART3_TXD,
+   MX51_PAD_EIM_D27__UART3_RTS,
+   MX51_PAD_EIM_D24__UART3_CTS,
+
+   /* I2C1 */
+   MX51_PAD_EIM_D16__I2C1_SDA,
+   MX51_PAD_EIM_D19__I2C1_SCL,
+
+   /* I2C2 */
+   MX51_PAD_KEY_COL4__I2C2_SCL,
+   MX51_PAD_KEY_COL5__I2C2_SDA,
+
+   /* HSI2C */
+   MX51_PAD_I2C1_CLK__I2C1_CLK,
+   MX51_PAD_I2C1_DAT__I2C1_DAT,
+
+   /* USB HOST1 */
+   MX51_PAD_USBH1_CLK__USBH1_CLK,
+   MX51_PAD_USBH1_DIR__USBH1_DIR,
+   MX51_PAD_USBH1_NXT__USBH1_NXT,
+   MX51_PAD_USBH1_DATA0__USBH1_DATA0,
+   MX51_PAD_USBH1_DATA1__USBH1_DATA1,
+   MX51_PAD_USBH1_DATA2__USBH1_DATA2,
+   MX51_PAD_USBH1_DATA3__USBH1_DATA3,
+   MX51_PAD_USBH1_DATA4__USBH1_DATA4,
+   MX51_PAD_USBH1_DATA5__USBH1_DATA5,
+   MX51_PAD_USBH1_DATA6__USBH1_DATA6,
+   MX51_PAD_USBH1_DATA7__USBH1_DATA7,
+
+   /* USB HUB reset line*/
+   MX51_PAD_GPIO1_7__GPIO1_7,
+
+   /* FEC */
+   MX51_PAD_EIM_EB2__FEC_MDIO,
+   MX51_PAD_EIM_EB3__FEC_RDATA1,
+   MX51_PAD_EIM_CS2__FEC_RDATA2,
+   MX51_PAD_EIM_CS3__FEC_RDATA3,
+   MX51_PAD_EIM_CS4__FEC_RX_ER,
+   MX51_PAD_EIM_CS5__FEC_CRS,
+   MX51_PAD_NANDF_RB2__FEC_COL,
+   MX51_PAD_NANDF_RB3__FEC_RX_CLK,
+   MX51_PAD_NANDF_D9__FEC_RDATA0,
+   MX51_PAD_NANDF_D8__FEC_TDATA0,
+   MX51_PAD_NANDF_CS2__FEC_TX_ER,
+   MX51_PAD_NANDF_CS3__FEC_MDC,
+   MX51_PAD_NANDF_CS4__FEC_TDATA1,
+   MX51_PAD_NANDF_CS5__FEC_TDATA2,
+   MX51_PAD_NANDF_CS6__FEC_TDATA3,
+   MX51_PAD_NANDF_CS7__FEC_TX_EN,
+   MX51_PAD_NANDF_RDY_INT__FEC_TX_CLK,
+
+   /* FEC PHY reset line */
+   MX51_PAD_EIM_A20__GPIO2_14,
+
+   /* SD 1 */
+   MX51_PAD_SD1_CMD__SD1_CMD,
+   MX51_PAD_SD1_CLK__SD1_CLK,
+   MX51_PAD_SD1_DATA0__SD1_DATA0,
+   MX51_PAD_SD1_DATA1__SD1_DATA1,
+   MX51_PAD_SD1_DATA2__SD1_DATA2,
+   MX51_PAD_SD1_DATA3__SD1_DATA3,
+
+   /* SD 2 */
+   MX51_PAD_SD2_CMD__SD2_CMD,
+   MX51_PAD_SD2_CLK__SD2_CLK,
+   MX51_PAD_SD2_DATA0__SD2_DATA0,
+   MX51_PAD_SD2_DATA1__SD2_DATA1,
+   MX51_PAD_SD2_DATA2__SD2_DATA2,
+   MX51_PAD_SD2_DATA3__SD2_DATA3,
+
+   /* eCSPI1 */
+   MX51_PAD_CSPI1_MISO__ECSPI1_MISO,
+   MX51_PAD_CSPI1_MOSI__ECSPI1_MOSI,
+   MX51_PAD_CSPI1_SCLK__ECSPI1_SCLK,
+   MX51_PAD_CSPI1_SS0__GPIO4_24,
+   MX51_PAD_CSPI1_SS1__GPIO4_25,
+};


This data already exists, so you should not duplicate it here.

Iomux setup is a good candidate for a DT binding as it is just data, but 
I never came up with a good solution that was not bloated with a 32-bit 
value for every setting of each pin.


Rob

___
linaro-dev mailing list
linaro-dev@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev


[PATCH] arm/dt: Add dtb make rule

2011-03-10 Thread Rob Herring
From: Rob Herring 

Add a make rule to compile dt blobs for ARM.

Signed-off-by: Rob Herring 
---
 arch/arm/Makefile  |3 +++
 arch/arm/boot/Makefile |6 ++
 2 files changed, 9 insertions(+), 0 deletions(-)

diff --git a/arch/arm/Makefile b/arch/arm/Makefile
index b49dea6..566763f 100644
--- a/arch/arm/Makefile
+++ b/arch/arm/Makefile
@@ -284,6 +284,9 @@ zImage Image xipImage bootpImage uImage: vmlinux
 zinstall install: vmlinux
$(Q)$(MAKE) $(build)=$(boot) MACHINE=$(MACHINE) $@
 
+%.dtb:
+   $(Q)$(MAKE) $(build)=$(boot) MACHINE=$(MACHINE) $(boot)/$@
+
 # We use MRPROPER_FILES and CLEAN_FILES now
 archclean:
$(Q)$(MAKE) $(clean)=$(boot)
diff --git a/arch/arm/boot/Makefile b/arch/arm/boot/Makefile
index dcf323d..bb63922 100644
--- a/arch/arm/boot/Makefile
+++ b/arch/arm/boot/Makefile
@@ -13,6 +13,8 @@
 
 MKIMAGE := $(srctree)/scripts/mkuboot.sh
 
+DTC_FLAGS := -p 1024
+
 ifneq ($(MACHINE),)
 include $(srctree)/$(MACHINE)/Makefile.boot
 endif
@@ -59,6 +61,10 @@ $(obj)/zImage:   $(obj)/compressed/vmlinux FORCE
 
 endif
 
+# Rule to build device tree blobs
+$(obj)/%.dtb: $(src)/dts/%.dts
+   $(call cmd,dtc)
+
 quiet_cmd_uimage = UIMAGE  $@
   cmd_uimage = $(CONFIG_SHELL) $(MKIMAGE) -A arm -O linux -T kernel \
   -C none -a $(LOADADDR) -e $(STARTADDR) \
-- 
1.7.1


___
linaro-dev mailing list
linaro-dev@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev


Re: Fw: [PATCH][RFC] CC_OPTIMIZE_FOR_SIZE should default to N

2011-03-07 Thread Rob Herring

On 03/07/2011 01:47 PM, Tom Gall wrote:

To quote the GCC manual:

-Os
Optimize for size. -Os enables all -O2 optimizations that do not
typically increase code size. It also performs further optimizations
designed to reduce code size.
-Os disables the following optimization flags:

   -falign-functions  -falign-jumps  -falign-loops
   -falign-labels  -freorder-blocks  -freorder-blocks-and-partition
   -fprefetch-loop-arrays  -ftree-vect-loop-version


That said (and unless there's other undocumented differences), it
would seem to take some expertise to comment on the trade off between
memory efforts (cache misses, TLB etc) vs instructional effects by
having these optimizations off.

I am certainly not that expert but I suspect given the memory bus
speeds that typical arm hardware has, we'd want -Os over -O2.



In my experience, -Os also turns off inlining of functions. The case I 
have looked at is swab32 function in u-boot. With -Os (and 
-march=armv7-a) I get calls to this function:


__fswab32:
rev r0, r0
bx lr

With -O2, I get rev inlined.

Rob


___
linaro-dev mailing list
linaro-dev@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev


Re: [PATCH 0/3] Add MX51 basic DT support

2011-02-18 Thread Rob Herring

Jason,

On 02/18/2011 02:12 AM, Jason Liu wrote:

This patchset adds FSL mx51 device tree support. This
is based on

git://git.secretlab.ca/git/linux-2.6 devicetree/test

This patch has been tested on MX51 babbage board and can
boot up succesfully to linux console with DT enabled.

Jason Liu (3):
   arm/dt: add basic mx51 device tree support
   arm/dt: add very basic dts file for babbage board


This is clearly based on the initial version I wrote. Can you give me 
credit for it.



   serial/imx: parse from device tree support


Jeremy Kerr wrote the initial version of this although it is quite 
heavily changed.


Original versions are here:

http://kernel.ubuntu.com/git?p=jk/dt/linux-2.6.git;a=shortlog;h=refs/heads/mx51

Rob

___
linaro-dev mailing list
linaro-dev@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev


Re: [PATCH 2/2] ARM: iMX5 BBG: add cpuidle driver parameters

2011-02-08 Thread Rob Herring

On 02/08/2011 09:51 AM, Yong Shen wrote:

Hi Arnaud,

I also took a while to think about this before posting patches. I prefer
to put it in board related code since the various PMIC used on each
boards may have influence on cpuidle latency or other charactors,
although it could be minor.



But you are not going to be doing voltage scaling in idle. Is it even 
possible to do sleeping operations like accessing a PMIC in idle?


The core is powergated, so lowering voltage would not help. Doing bus 
scaling or DDR self-refresh are the only likely additional operations.


Rob

___
linaro-dev mailing list
linaro-dev@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev


Re: [PATCH 4/4] watchdog: s3c2410: Add support for device tree based probe

2011-02-07 Thread Rob Herring

Thomas,

On 02/06/2011 07:17 AM, Thomas Abraham wrote:

This patch adds the of_match_table to enable s3c2410-wdt driver
to be probed when watchdog device node is found in the device tree.

Signed-off-by: Thomas Abraham
---
  drivers/watchdog/s3c2410_wdt.c |   10 ++
  1 files changed, 10 insertions(+), 0 deletions(-)

diff --git a/drivers/watchdog/s3c2410_wdt.c b/drivers/watchdog/s3c2410_wdt.c
index ae53662..a9edd50 100644
--- a/drivers/watchdog/s3c2410_wdt.c
+++ b/drivers/watchdog/s3c2410_wdt.c
@@ -592,6 +592,13 @@ static int s3c2410wdt_resume(struct platform_device *dev)
  #define s3c2410wdt_resume  NULL
  #endif /* CONFIG_PM */

+#ifdef CONFIG_OF
+static const struct of_device_id s3c2410_wdt_match[] = {
+   { .compatible = "samsung,s3c2410-wdt" },
+   {},
+};
+MODULE_DEVICE_TABLE(of, s3c2410_wdt_match);
+#endif

  static struct platform_driver s3c2410wdt_driver = {
.probe  = s3c2410wdt_probe,
@@ -602,6 +609,9 @@ static struct platform_driver s3c2410wdt_driver = {
.driver = {
.owner  = THIS_MODULE,
.name   = "s3c2410-wdt",
+#ifdef CONFIG_OF
+   .of_match_table = s3c2410_wdt_match,
+#endif


The ifdefs aren't necessary if you pick-up Grant's patch in 
devicetree/next branch.


Rob


___
linaro-dev mailing list
linaro-dev@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev


Re: [PATCH 2/4] ARM: DT: Add a basic dts file for SMDKV310 machine

2011-02-07 Thread Rob Herring

David, Thomas,

On 02/06/2011 06:04 PM, David Gibson wrote:

On Sun, Feb 06, 2011 at 06:47:28PM +0530, Thomas Abraham wrote:

This patch adds a basic dts file for Samsung's SMDKV310 machine.

Signed-off-by: Thomas Abraham
---
  arch/arm/mach-s5pv310/mach-smdkv310.dts |   38 +++
  1 files changed, 38 insertions(+), 0 deletions(-)
  create mode 100755 arch/arm/mach-s5pv310/mach-smdkv310.dts

diff --git a/arch/arm/mach-s5pv310/mach-smdkv310.dts 
b/arch/arm/mach-s5pv310/mach-smdkv310.dts
new file mode 100755
index 000..74d80bf
--- /dev/null
+++ b/arch/arm/mach-s5pv310/mach-smdkv310.dts
@@ -0,0 +1,38 @@
+/dts-v1/;
+
+/ {
+   model = "smdkv310";
+   compatible = "samsung,smdkv310";
+   #address-cells =<1>;
+   #size-cells =<1>;
+
+   memory {
+   device_type = "memory";
+   reg =<0x4000 0x0800>;
+   };


Uh.. where are the cpus?



But for ARM, all the details of the cpu are probe-able. So what would we 
gain by putting cpu info in the DTS?



+   chosen {
+   bootargs = "root=/dev/ram0 rw ramdisk=8192 initrd=0x4100,8M 
console=ttySAC1,115200 init=/linuxrc";
+   };
+
+   soc {
+   #address-cells =<1>;
+   #size-cells =<1>;
+   compatible = "simple-bus";


It's generally a good idea to list the specific soc model before "simple-bus".


+   ranges =<0x 0x 0x>;


For no translation, you can do just:

ranges;

Rob

___
linaro-dev mailing list
linaro-dev@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev


Re: [RFC PATCH] SDHCI: S3C: Add support for retrieving memory and irq resource information from device tree.

2011-01-31 Thread Rob Herring

Thomas,

On 01/31/2011 10:28 AM, thomas.abra...@linaro.org wrote:

From: Thomas Abraham

Add support for retrieving memory and irq resource information
from device tree for Samsung's SDHCI controller driver.

Signed-off-by: Thomas Abraham
---

The modification will be made more generic to support both
DT and non-DT versions of the driver without the #ifdef's.
For now, this patch is for review and to understand if the
approach adopted to obtain resource information from the
device tree is appropriate.



There is already an OF SDHCI driver. Some fixes from me for ARM have 
gone into .38.



  drivers/mmc/host/sdhci-s3c.c |   32 
  1 files changed, 32 insertions(+), 0 deletions(-)

diff --git a/drivers/mmc/host/sdhci-s3c.c b/drivers/mmc/host/sdhci-s3c.c
index 1720358..f536061 100644
--- a/drivers/mmc/host/sdhci-s3c.c
+++ b/drivers/mmc/host/sdhci-s3c.c
@@ -19,6 +19,9 @@
  #include
  #include
  #include
+#include
+#include
+#include

  #include

@@ -348,23 +351,52 @@ static int __devinit sdhci_s3c_probe(struct 
platform_device *pdev)
struct sdhci_s3c *sc;
struct resource *res;
int ret, irq, ptr, clks;
+   struct device_node *np = NULL;
+#ifdef CONFIG_OF
+   struct resource iores;
+#endif

if (!pdata) {
dev_err(dev, "no device data specified\n");
return -ENOENT;
}

+#ifdef CONFIG_OF
+   for_each_compatible_node(np, NULL, "samsung,sdhci-s3c") {
+   const u32 *id = of_get_property(np, "cell-index", NULL);


Per Grant, using cell-index should be avoided.


+   if (be32_to_cpu(*id) == pdev->id)


Any drivers that depend on pdev->id being an index will break for device 
tree.



+   break;
+   }
+
+   if (!np) {
+   dev_err(dev, "no matching device node specified in device 
tree\n");
+   return -ENOENT;
+   }
+#endif
+
+#ifndef CONFIG_OF
irq = platform_get_irq(pdev, 0);


Using platform_get_irq works for OF drivers too.


+#else
+   irq = of_irq_to_resource(np, 0, NULL);
+#endif
if (irq<  0) {
dev_err(dev, "no irq specified\n");
return irq;
}

+#ifndef CONFIG_OF
res = platform_get_resource(pdev, IORESOURCE_MEM, 0);


Ditto


if (!res) {
dev_err(dev, "no memory specified\n");
return -ENOENT;
}
+#else
+   if (of_address_to_resource(np, 0,&iores)) {
+   dev_err(dev, "no memory specified in device tree\n");
+   return -ENOENT;
+   }
+   res =&iores;
+#endif

host = sdhci_alloc_host(dev, sizeof(struct sdhci_s3c));
if (IS_ERR(host)) {


You are missing an of_match_table. This patch could not work without it.

The primary things you need to do for OF support on a driver are add the 
match table and convert platform data to OF bindings.


Rob

___
linaro-dev mailing list
linaro-dev@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev


Re: [RFC] arm devicetree meeting/hacking day

2011-01-25 Thread Rob Herring

Grant,

On 01/24/2011 02:25 PM, Grant Likely wrote:

Hi all,

For those of you who are working, or are soon-to-be working, on device
tree support for ARM, I'm organizing a regular weekly status update
and a hacking day for getting device tree support added to several of
the ARM platforms.  I've already talked to some of you about this
briefly.  Basically the idea is to have a day when all of us working
on dt can coordinate and get problems solved.  I'll commit to being
available and focused on answering dt questions for that day so it
should be easier to get my help.  I'll also put together some
documents and guides for dt bringup on new platforms which will break
down the features that each platform needs to implement.

For the status update part, rather than scheduling yet another
meeting, I think it would be appropriate to tack it onto the end of
the Monday kernel working group status update call.  I'll dial into
both the west (1600UTC) and east (0400UTC-tuesday) calls so that I've
got a chance to talk with everyone..  Let me know if you don't have
the dial-in details for either of the calls and I'll ask Paul or Loic
to add you to the invite.

For the hacking day, how does Tuesday(west)/Wednesday(east) sound for
everyone?  I'll be available on IRC, and I'll also keep my schedule
clear so that I'll be available for phone calls.

Right now, here's the list of people that I know are or may be working
on DT support, and the platform they are working on (not exclusively
Linaro work):

Lennert Buytenhek (Core infrastructure&  omap3)
Thomas Abraham (Samsung)
John Bonesio (nVidia Tegra)
Shawn Guo and Jason Hui (Freescale imx51)
Jon Masters (omap3, beagle board)

Anybody else want me to add them to this list?


Rob Herring (Calxeda)

I can help out on testing and continue providing any fixes I find.

Rob

___
linaro-dev mailing list
linaro-dev@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev


Re: [PATCH][ ARM cpu hotplug 1/2 ] extract common code for arm cpu hotplug

2010-11-30 Thread Rob Herring

On 11/30/2010 05:03 AM, Russell King - ARM Linux wrote:

On Tue, Nov 30, 2010 at 04:17:32PM +0530, Amit Kucheria wrote:

Since the main aim here is to consolidate as much code here as
possible while still allowing platforms to override the defaults,
would you have an objection to the introduction of a struct smp_ops
that'll allow a platform to override the defaults? This seems to be
done on other platforms I've briefly looked at.


I see no point to what is being proposed in this thread.  It's _soo_
little code that the platforms have to implement that it really is
not worth the effort.

Whether the code can be consolidated or not, the current API does not 
allow a single kernel binary (ignoring the list of other issues). The 
introduction of struct smp_ops would help enable both single kernel and 
default versions of the functions.


Rob

___
linaro-dev mailing list
linaro-dev@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev


Re: NEON power consumption

2010-11-30 Thread Rob Herring

On 11/30/2010 03:41 AM, Dave Martin wrote:

Hi,

On Mon, Nov 29, 2010 at 8:45 PM, Michael Hope  wrote:

On Tue, Nov 30, 2010 at 12:37 AM, Dave Martin  wrote:

On Sun, Nov 28, 2010 at 10:28 PM, Michael Hope  wrote:

I sat down and measured the power consumption of the NEON unit on an
OMAP3.  Method and results are here:
  https://wiki.linaro.org/MichaelHope/Sandbox/NEONPower

The board takes 2.37 W and the NEON unit adds an extra 120 mW.
Assuming the core takes 1 W, then the code needs to run 12 % faster
with NEON on to be a net power win.

Note that the results are inaccurate but valid enough.


Just to play devil's advocate... the results will differ, perhaps
significantly, between SoCs of course.

In terms of the amount of energy required to perform a particular
operation (i.e., at the microbenchmark level) I agree with your
conclusion.  However, in practice I suspect this isn't enough.  I'm
not familiar with exactly when NEON is likely to get turned on and
off, but you need to factor in the behaviour of the OS--- if you
accelerate a DSP operation which is used a few dozen times per
timeslice, NEON will be used for only a tiny proportion of the time it
is used, because once NEON is on, it probably stays on at least until
the interrupt, and probably until the next task switch.  With the
kernel configured for dynamic timer tick, this can get even more
exaggerated, since the rescheduling frequency may drop.

The real benefits, in performance and power, therefore come in
operations which dominate the run-time of a particular process, such
as intensive image handling or codec operations.  NEON in
widely-dispersed but sporadically used features (such as
general-purpose library code) could be expected to come at a net power
cost.  If you use NEON for memcpy for example, you will basically
never be able to turn the NEON unit off.  That's unlikely to be a win
overall, since even if you now optimise all the code in the system for
NEON, you're unlikely to see a significant performance boost-- NEON
simply isn't designed for accelerating general-purpose code.

The correct decision for how to optimise a given piece of code seems
to depend on the SoC and the runtime load profile.  And while you can
usefully predict that at build-time for a media player or dedicated
media stack components, it's pretty much impossible to do so with
general-purpose libraries... unless there's a cunning strategy I
haven't thought of.

Ideally, processes whose load varies significantly over time and
between different use cases (such as Xorg) would be able to select
between NEON-ised and non-NEON-ised implementations dynamically, based
on the current load.  But I guess we're some distance away from being
able to achieve that... ?


I agree.  I've been wondering if this is more of a power management
topic as what you've described there is basically the same as what the
CPU frequency governor does in deciding the best way to achieve a
workload.  Perhaps this can also turn into hints to executing code re:
what instruction set to use.

There might be an argument for explicit control as well.  Say you're
decoding a AAC stream and using 20 % CPU - it might be more efficient
to acquire and release the NEON unit from within the decoder to start
it up faster and release it as soon as the job is done.

Could a kernel developer describe how the NEON unit is controlled?  My
understanding is:
  * NEON is generally off
  * Executing a NEON instruction causes a instruction trap, which kicks
the kernel, which starts the unit up
  * The kernel only saves the NEON registers if the code uses them


I'll give the architectural view--- someone else will have to comment
on the hardware.

Currently, at every context switch, the kernel disables VFP and NEON
by clearing the EN bit in the FPEXC control register.  The first
attempt use use VFP or NEON by the process will cause a trap into the
kernel, which does any necessary context switching of the VFP/NEON
registers, enables them by setting FPEXC.EN and returning to
userspace.  VFP and NEON remain enabled until the next context switch.

This policy has nothing to do with power--- it's purely done so that
the VFP and NEON context can be switched lazily.  If the kernel
switches to a process that doesn't use VFP or NEON, the old register
contents will remain, so you may also save an additional register bank
context switch if the next context switch takes you back to the
process which actually owns the register contents.

Particular SoCs may implement their own additional stragety for power
management.  A particular SoC may respond to the toggling of FPEXC.EN
by clock-gating the whole NEON functional unit for example.  Or there
may some entirely separate logic.  However, in the current
implementation I believe the NEON unit can't normally be destructively
powered down, since the kernel assumes that the last register contents
switched into the VFP/NEON register bank are preserved.



On SMP, the registers are saved on c

Re: Rough notes from Kernel Consolidation meeting

2010-09-21 Thread Rob Herring
On 09/21/2010 11:02 AM, Christian Robottom Reis wrote:
> On Tue, Sep 21, 2010 at 12:15:17AM +0200, Robert Schwebel wrote:
>> On Mon, Sep 20, 2010 at 09:19:49AM -0700, Paul E. McKenney wrote:
>>> o   Jason Hui: iMX51 work on device trees.  Need assignment.
>>> Reviewed BSP review, need to send results to Loïc et al.
>>
>> It would be great to see more MX51 things on ALKML (and devicetree
>> things on devtree-discuss). Up to now I have the impression that there
>> is still much work done behind closed doors, which is bad if we want to
>> have better mainline support for i.MX5x.
>
> That's a good point and there's nothing behind closed doors happening
> that I have requested -- in fact, I mandate the opposite. I would assume
> that Jeremy has been posting device tree mx51 work to the
> devtree-discuss ML, and that mx51 patches coming out of the KWG and PMWG
> are also posted to ALKML; correct me if I'm wrong.
>
> It may be that it's just very little happening :-/

In fact, that is the case. There has been no work on it in 4 months. The 
main area of work since then has been on a common struct clk to enable 
DT clock support.

The MX51 DT work is here:

http://kernel.ubuntu.com/git?p=jk/dt/linux-2.6.git;a=shortlog;h=refs/heads/mx51

Rob

___
linaro-dev mailing list
linaro-dev@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev