[RFC 08/11] ARM: dts: am33xx: Add prcm_resets node
Add the prcm_resets node to the prcm parent node.
Add the dt-bindings header to the DT file
Signed-off-by: Dan Murphy
---
arch/arm/boot/dts/am33xx.dtsi|6 ++
include/dt-bindings/reset/ti,am33xx-resets.h | 18 ++
2 files changed, 24 insertions(+)
create mode 100644 include/dt-bindings/reset/ti,am33xx-resets.h
diff --git a/arch/arm/boot/dts/am33xx.dtsi b/arch/arm/boot/dts/am33xx.dtsi
index 07f283c..df9d9c6 100644
--- a/arch/arm/boot/dts/am33xx.dtsi
+++ b/arch/arm/boot/dts/am33xx.dtsi
@@ -10,6 +10,7 @@
#include
#include
+#include
#include "skeleton.dtsi"
@@ -117,6 +118,11 @@
prcm_clockdomains: clockdomains {
};
+
+ prcm_resets: resets {
+ compatible = "ti,am335x-resets";
+ #reset-cells = <1>;
+ };
};
scrm: scrm@44e1 {
diff --git a/include/dt-bindings/reset/ti,am33xx-resets.h
b/include/dt-bindings/reset/ti,am33xx-resets.h
new file mode 100644
index 000..dfe7954
--- /dev/null
+++ b/include/dt-bindings/reset/ti,am33xx-resets.h
@@ -0,0 +1,18 @@
+/*
+ * AM33xx reset index for PRCM Module
+ *
+ * Copyright 2014 Texas Instruments Inc.
+ *
+ * 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.
+ */
+
+#ifndef _DT_BINDINGS_RESET_TI_AM33XX_H
+#define _DT_BINDINGS_RESET_TI_AM33XX_H
+
+#define RESET_DEVICE_RESET 0
+#define RESET_GFX_RESET1
+#define RESET_PER_RESET2
+
+#endif
--
1.7.9.5
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC 08/11] ARM: dts: am33xx: Add prcm_resets node
On Tuesday 29 April 2014 15:19:47 Dan Murphy wrote: > + * AM33xx reset index for PRCM Module > + * > + * Copyright 2014 Texas Instruments Inc. > + * > + * 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. > + */ > + > +#ifndef _DT_BINDINGS_RESET_TI_AM33XX_H > +#define _DT_BINDINGS_RESET_TI_AM33XX_H > + > +#define RESET_DEVICE_RESET 0 > +#define RESET_GFX_RESET1 > +#define RESET_PER_RESET2 > + > +#endif Interfaces like this should only be used if you can't use hardware numbers, in general. If these numbers are in the data sheet, just put them directly into the dts file, as we do for interrupt numbers, gpio numbers, register offsets etc. If you have made them up to define an interface between the driver and DT because there is no usable hardare ID, I'd suggest just using a single file across all SoCs that have this driver, and have a unified name space. Arnd -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC 08/11] ARM: dts: am33xx: Add prcm_resets node
* Arnd Bergmann [140429 13:35]:
> On Tuesday 29 April 2014 15:19:47 Dan Murphy wrote:
> > + * AM33xx reset index for PRCM Module
> > + *
> > + * Copyright 2014 Texas Instruments Inc.
> > + *
> > + * 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.
> > + */
> > +
> > +#ifndef _DT_BINDINGS_RESET_TI_AM33XX_H
> > +#define _DT_BINDINGS_RESET_TI_AM33XX_H
> > +
> > +#define RESET_DEVICE_RESET 0
> > +#define RESET_GFX_RESET1
> > +#define RESET_PER_RESET2
> > +
> > +#endif
>
> Interfaces like this should only be used if you can't use hardware
> numbers, in general. If these numbers are in the data sheet, just
> put them directly into the dts file, as we do for interrupt numbers,
> gpio numbers, register offsets etc.
>
> If you have made them up to define an interface between the driver
> and DT because there is no usable hardare ID, I'd suggest just using
> a single file across all SoCs that have this driver, and have
> a unified name space.
Also, it's a bit unclear how the reset controller phandle is used
referenced and used by the consumer device.. Maybe setting that up
first in a Linux generic way is a good point starting point.
Maybe something like this along the same way as clocks are set up
(completely untested):
&reset1 {
iva_reset: reset1 {
reg = /bits/ 8 <0>;
};
gfx_reset: reset1 {
reg = /bits/ 8 <1>;
};
...
};
&iva {
compatible = "ti,ivahd";
resets = <&reset1 1>;
...
};
Regards,
Tony
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC 08/11] ARM: dts: am33xx: Add prcm_resets node
Tony and Arnd
Thanks for the comments
On 04/29/2014 07:22 PM, Tony Lindgren wrote:
> * Arnd Bergmann [140429 13:35]:
>> On Tuesday 29 April 2014 15:19:47 Dan Murphy wrote:
>>> + * AM33xx reset index for PRCM Module
>>> + *
>>> + * Copyright 2014 Texas Instruments Inc.
>>> + *
>>> + * 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.
>>> + */
>>> +
>>> +#ifndef _DT_BINDINGS_RESET_TI_AM33XX_H
>>> +#define _DT_BINDINGS_RESET_TI_AM33XX_H
>>> +
>>> +#define RESET_DEVICE_RESET 0
>>> +#define RESET_GFX_RESET1
>>> +#define RESET_PER_RESET2
>>> +
>>> +#endif
>> Interfaces like this should only be used if you can't use hardware
>> numbers, in general. If these numbers are in the data sheet, just
>> put them directly into the dts file, as we do for interrupt numbers,
>> gpio numbers, register offsets etc.
>>
>> If you have made them up to define an interface between the driver
>> and DT because there is no usable hardare ID, I'd suggest just using
>> a single file across all SoCs that have this driver, and have
>> a unified name space.
> Also, it's a bit unclear how the reset controller phandle is used
> referenced and used by the consumer device.. Maybe setting that up
> first in a Linux generic way is a good point starting point.
>
> Maybe something like this along the same way as clocks are set up
> (completely untested):
>
> &reset1 {
> iva_reset: reset1 {
> reg = /bits/ 8 <0>;
> };
> gfx_reset: reset1 {
> reg = /bits/ 8 <1>;
> };
> ...
> };
>
> &iva {
> compatible = "ti,ivahd";
> resets = <&reset1 1>;
> ...
> };
I had something very similar to this when I was developing this driver but
moved away from this.
Following the clocks implementation I had a separate dtsi for resets for each
device and had the data defined like so
for each SoC.
&prcm_resets {
device_reset: device_reset {
rstctrl_offs = <0x1104>;
ctrl_bit-shift = <0>;
rstst_offs = <0x1114>;
sts_bit-shift = <0>;
};
gpu_reset: gpu_reset {
rstctrl_offs = <0x0D00>;
ctrl_bit-shift = <3>;
rstst_offs = <0x0D0C>;
sts_bit-shift = <5>;
};
};
And then any client interested in a specific reset driver would include this
resets = <&prcm_resets &gpu_reset>;
reset-names = "gpu_reset";
Our reset code would then retrieve the register data through the phandle
instead of an index.
Thoughts?
Dan
> Regards,
>
> Tony
--
--
Dan Murphy
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC 08/11] ARM: dts: am33xx: Add prcm_resets node
* Dan Murphy [140430 11:00]:
> Tony and Arnd
>
> Thanks for the comments
>
> On 04/29/2014 07:22 PM, Tony Lindgren wrote:
> > * Arnd Bergmann [140429 13:35]:
> >> On Tuesday 29 April 2014 15:19:47 Dan Murphy wrote:
> >>> + * AM33xx reset index for PRCM Module
> >>> + *
> >>> + * Copyright 2014 Texas Instruments Inc.
> >>> + *
> >>> + * 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.
> >>> + */
> >>> +
> >>> +#ifndef _DT_BINDINGS_RESET_TI_AM33XX_H
> >>> +#define _DT_BINDINGS_RESET_TI_AM33XX_H
> >>> +
> >>> +#define RESET_DEVICE_RESET 0
> >>> +#define RESET_GFX_RESET1
> >>> +#define RESET_PER_RESET2
> >>> +
> >>> +#endif
> >> Interfaces like this should only be used if you can't use hardware
> >> numbers, in general. If these numbers are in the data sheet, just
> >> put them directly into the dts file, as we do for interrupt numbers,
> >> gpio numbers, register offsets etc.
> >>
> >> If you have made them up to define an interface between the driver
> >> and DT because there is no usable hardare ID, I'd suggest just using
> >> a single file across all SoCs that have this driver, and have
> >> a unified name space.
> > Also, it's a bit unclear how the reset controller phandle is used
> > referenced and used by the consumer device.. Maybe setting that up
> > first in a Linux generic way is a good point starting point.
> >
> > Maybe something like this along the same way as clocks are set up
> > (completely untested):
> >
> > &reset1 {
> > iva_reset: reset1 {
> > reg = /bits/ 8 <0>;
> > };
> > gfx_reset: reset1 {
> > reg = /bits/ 8 <1>;
> > };
> > ...
> > };
> >
> > &iva {
> > compatible = "ti,ivahd";
> > resets = <&reset1 1>;
> > ...
> > };
>
> I had something very similar to this when I was developing this driver but
> moved away from this.
>
> Following the clocks implementation I had a separate dtsi for resets for each
> device and had the data defined like so
> for each SoC.
>
> &prcm_resets {
>device_reset: device_reset {
>rstctrl_offs = <0x1104>;
>ctrl_bit-shift = <0>;
>rstst_offs = <0x1114>;
>sts_bit-shift = <0>;
>};
>
>gpu_reset: gpu_reset {
>rstctrl_offs = <0x0D00>;
>ctrl_bit-shift = <3>;
>rstst_offs = <0x0D0C>;
>sts_bit-shift = <5>;
>};
> };
>
> And then any client interested in a specific reset driver would include this
>
> resets = <&prcm_resets &gpu_reset>;
> reset-names = "gpu_reset";
>
> Our reset code would then retrieve the register data through the phandle
> instead of an index.
>
> Thoughts?
Using phandles makes sense here because it avoids the indexing. Indexing
has a problem of needing to be in sync with the .dts files and the
device driver(s). Using an index also easily causes misuse of virtual
indexes being added that no longer describe the hardware at all.
Regards,
Tony
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC 08/11] ARM: dts: am33xx: Add prcm_resets node
Tony
On 04/30/2014 01:10 PM, Tony Lindgren wrote:
> * Dan Murphy [140430 11:00]:
>> Tony and Arnd
>>
>> Thanks for the comments
>>
>> On 04/29/2014 07:22 PM, Tony Lindgren wrote:
>>> * Arnd Bergmann [140429 13:35]:
On Tuesday 29 April 2014 15:19:47 Dan Murphy wrote:
> + * AM33xx reset index for PRCM Module
> + *
> + * Copyright 2014 Texas Instruments Inc.
> + *
> + * 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.
> + */
> +
> +#ifndef _DT_BINDINGS_RESET_TI_AM33XX_H
> +#define _DT_BINDINGS_RESET_TI_AM33XX_H
> +
> +#define RESET_DEVICE_RESET 0
> +#define RESET_GFX_RESET1
> +#define RESET_PER_RESET2
> +
> +#endif
Interfaces like this should only be used if you can't use hardware
numbers, in general. If these numbers are in the data sheet, just
put them directly into the dts file, as we do for interrupt numbers,
gpio numbers, register offsets etc.
If you have made them up to define an interface between the driver
and DT because there is no usable hardare ID, I'd suggest just using
a single file across all SoCs that have this driver, and have
a unified name space.
>>> Also, it's a bit unclear how the reset controller phandle is used
>>> referenced and used by the consumer device.. Maybe setting that up
>>> first in a Linux generic way is a good point starting point.
>>>
>>> Maybe something like this along the same way as clocks are set up
>>> (completely untested):
>>>
>>> &reset1 {
>>> iva_reset: reset1 {
>>> reg = /bits/ 8 <0>;
>>> };
>>> gfx_reset: reset1 {
>>> reg = /bits/ 8 <1>;
>>> };
>>> ...
>>> };
>>>
>>> &iva {
>>> compatible = "ti,ivahd";
>>> resets = <&reset1 1>;
>>> ...
>>> };
>> I had something very similar to this when I was developing this driver but
>> moved away from this.
>>
>> Following the clocks implementation I had a separate dtsi for resets for
>> each device and had the data defined like so
>> for each SoC.
>>
>> &prcm_resets {
>>device_reset: device_reset {
>>rstctrl_offs = <0x1104>;
>>ctrl_bit-shift = <0>;
>>rstst_offs = <0x1114>;
>>sts_bit-shift = <0>;
>>};
>>
>>gpu_reset: gpu_reset {
>>rstctrl_offs = <0x0D00>;
>>ctrl_bit-shift = <3>;
>>rstst_offs = <0x0D0C>;
>>sts_bit-shift = <5>;
>>};
>> };
>>
>> And then any client interested in a specific reset driver would include this
>>
>> resets = <&prcm_resets &gpu_reset>;
>> reset-names = "gpu_reset";
>>
>> Our reset code would then retrieve the register data through the phandle
>> instead of an index.
>>
>> Thoughts?
> Using phandles makes sense here because it avoids the indexing. Indexing
> has a problem of needing to be in sync with the .dts files and the
> device driver(s). Using an index also easily causes misuse of virtual
> indexes being added that no longer describe the hardware at all.
Thanks. What about placing register data in the dts files? Is there any issue
with this concept?
Dan
> Regards,
>
> Tony
--
--
Dan Murphy
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC 08/11] ARM: dts: am33xx: Add prcm_resets node
* Dan Murphy [140430 11:14]:
> On 04/30/2014 01:10 PM, Tony Lindgren wrote:
> > * Dan Murphy [140430 11:00]:
> >> Tony and Arnd
> >>
> >> Thanks for the comments
> >>
> >> On 04/29/2014 07:22 PM, Tony Lindgren wrote:
> >>> * Arnd Bergmann [140429 13:35]:
> On Tuesday 29 April 2014 15:19:47 Dan Murphy wrote:
> > + * AM33xx reset index for PRCM Module
> > + *
> > + * Copyright 2014 Texas Instruments Inc.
> > + *
> > + * 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.
> > + */
> > +
> > +#ifndef _DT_BINDINGS_RESET_TI_AM33XX_H
> > +#define _DT_BINDINGS_RESET_TI_AM33XX_H
> > +
> > +#define RESET_DEVICE_RESET 0
> > +#define RESET_GFX_RESET1
> > +#define RESET_PER_RESET2
> > +
> > +#endif
> Interfaces like this should only be used if you can't use hardware
> numbers, in general. If these numbers are in the data sheet, just
> put them directly into the dts file, as we do for interrupt numbers,
> gpio numbers, register offsets etc.
>
> If you have made them up to define an interface between the driver
> and DT because there is no usable hardare ID, I'd suggest just using
> a single file across all SoCs that have this driver, and have
> a unified name space.
> >>> Also, it's a bit unclear how the reset controller phandle is used
> >>> referenced and used by the consumer device.. Maybe setting that up
> >>> first in a Linux generic way is a good point starting point.
> >>>
> >>> Maybe something like this along the same way as clocks are set up
> >>> (completely untested):
> >>>
> >>> &reset1 {
> >>> iva_reset: reset1 {
> >>> reg = /bits/ 8 <0>;
> >>> };
> >>> gfx_reset: reset1 {
> >>> reg = /bits/ 8 <1>;
> >>> };
> >>> ...
> >>> };
> >>>
> >>> &iva {
> >>> compatible = "ti,ivahd";
> >>> resets = <&reset1 1>;
> >>> ...
> >>> };
> >> I had something very similar to this when I was developing this driver but
> >> moved away from this.
> >>
> >> Following the clocks implementation I had a separate dtsi for resets for
> >> each device and had the data defined like so
> >> for each SoC.
> >>
> >> &prcm_resets {
> >>device_reset: device_reset {
> >>rstctrl_offs = <0x1104>;
> >>ctrl_bit-shift = <0>;
> >>rstst_offs = <0x1114>;
> >>sts_bit-shift = <0>;
> >>};
> >>
> >>gpu_reset: gpu_reset {
> >>rstctrl_offs = <0x0D00>;
> >>ctrl_bit-shift = <3>;
> >>rstst_offs = <0x0D0C>;
> >>sts_bit-shift = <5>;
> >>};
> >> };
> >>
> >> And then any client interested in a specific reset driver would include
> >> this
> >>
> >> resets = <&prcm_resets &gpu_reset>;
> >> reset-names = "gpu_reset";
> >>
> >> Our reset code would then retrieve the register data through the phandle
> >> instead of an index.
> >>
> >> Thoughts?
> > Using phandles makes sense here because it avoids the indexing. Indexing
> > has a problem of needing to be in sync with the .dts files and the
> > device driver(s). Using an index also easily causes misuse of virtual
> > indexes being added that no longer describe the hardware at all.
>
> Thanks. What about placing register data in the dts files? Is there any
> issue with this concept?
I don't have issues with that but others may. In this case it seems
like you should get away just defining few different types of reset
controllers without adding any any custom properties?
In your example above, the rstctrl_offs should be just standard
reg entry as an offset from the prcm_resets base address. Then you
you only really need the bit shift and the type of the reset
controller? And the type could be the compatible flag?
Regards,
Tony
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC 08/11] ARM: dts: am33xx: Add prcm_resets node
Tony
On 04/30/2014 05:33 PM, Tony Lindgren wrote:
> * Dan Murphy [140430 11:14]:
>> On 04/30/2014 01:10 PM, Tony Lindgren wrote:
>>> * Dan Murphy [140430 11:00]:
Tony and Arnd
Thanks for the comments
On 04/29/2014 07:22 PM, Tony Lindgren wrote:
> * Arnd Bergmann [140429 13:35]:
>> On Tuesday 29 April 2014 15:19:47 Dan Murphy wrote:
>>> + * AM33xx reset index for PRCM Module
>>> + *
>>> + * Copyright 2014 Texas Instruments Inc.
>>> + *
>>> + * 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.
>>> + */
>>> +
>>> +#ifndef _DT_BINDINGS_RESET_TI_AM33XX_H
>>> +#define _DT_BINDINGS_RESET_TI_AM33XX_H
>>> +
>>> +#define RESET_DEVICE_RESET 0
>>> +#define RESET_GFX_RESET1
>>> +#define RESET_PER_RESET2
>>> +
>>> +#endif
>> Interfaces like this should only be used if you can't use hardware
>> numbers, in general. If these numbers are in the data sheet, just
>> put them directly into the dts file, as we do for interrupt numbers,
>> gpio numbers, register offsets etc.
>>
>> If you have made them up to define an interface between the driver
>> and DT because there is no usable hardare ID, I'd suggest just using
>> a single file across all SoCs that have this driver, and have
>> a unified name space.
> Also, it's a bit unclear how the reset controller phandle is used
> referenced and used by the consumer device.. Maybe setting that up
> first in a Linux generic way is a good point starting point.
>
> Maybe something like this along the same way as clocks are set up
> (completely untested):
>
> &reset1 {
> iva_reset: reset1 {
> reg = /bits/ 8 <0>;
> };
> gfx_reset: reset1 {
> reg = /bits/ 8 <1>;
> };
> ...
> };
>
> &iva {
> compatible = "ti,ivahd";
> resets = <&reset1 1>;
> ...
> };
I had something very similar to this when I was developing this driver but
moved away from this.
Following the clocks implementation I had a separate dtsi for resets for
each device and had the data defined like so
for each SoC.
&prcm_resets {
device_reset: device_reset {
rstctrl_offs = <0x1104>;
ctrl_bit-shift = <0>;
rstst_offs = <0x1114>;
sts_bit-shift = <0>;
};
gpu_reset: gpu_reset {
rstctrl_offs = <0x0D00>;
ctrl_bit-shift = <3>;
rstst_offs = <0x0D0C>;
sts_bit-shift = <5>;
};
};
And then any client interested in a specific reset driver would include
this
resets = <&prcm_resets &gpu_reset>;
reset-names = "gpu_reset";
Our reset code would then retrieve the register data through the phandle
instead of an index.
Thoughts?
>>> Using phandles makes sense here because it avoids the indexing. Indexing
>>> has a problem of needing to be in sync with the .dts files and the
>>> device driver(s). Using an index also easily causes misuse of virtual
>>> indexes being added that no longer describe the hardware at all.
>> Thanks. What about placing register data in the dts files? Is there any
>> issue with this concept?
> I don't have issues with that but others may. In this case it seems
> like you should get away just defining few different types of reset
> controllers without adding any any custom properties?
>
> In your example above, the rstctrl_offs should be just standard
> reg entry as an offset from the prcm_resets base address. Then you
> you only really need the bit shift and the type of the reset
> controller? And the type could be the compatible flag?
Well the only issue I have with declaring the reg with the standard dt entry
is I have two register offsets here.
I will have to see if I can do this per the standard.
I may have to have children called control and status and define the reg and
bits that way.
Dan
> Regards,
>
> Tony
>
--
--
Dan Murphy
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
