Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1423158 > unrolled thread
| Started by | "Andrew F. Davis" <afd@ti.com> |
|---|---|
| First post | 2016-06-15 17:50 +0200 |
| Last post | 2016-06-20 21:10 +0200 |
| Articles | 4 — 3 participants |
Back to article view | Back to linux.kernel
This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by
below is the oldest one visible, not the original post.
Re: [PATCH v3 0/2] Add support for SYSCON reset "Andrew F. Davis" <afd@ti.com> - 2016-06-15 17:50 +0200
Re: [PATCH v3 0/2] Add support for SYSCON reset Philipp Zabel <p.zabel@pengutronix.de> - 2016-06-15 18:50 +0200
Re: [PATCH v3 0/2] Add support for SYSCON reset Rob Herring <robh+dt@kernel.org> - 2016-06-16 04:20 +0200
Re: [PATCH v3 0/2] Add support for SYSCON reset "Andrew F. Davis" <afd@ti.com> - 2016-06-20 21:10 +0200
| From | "Andrew F. Davis" <afd@ti.com> |
|---|---|
| Date | 2016-06-15 17:50 +0200 |
| Subject | Re: [PATCH v3 0/2] Add support for SYSCON reset |
| Message-ID | <rKleO-4pE-11@gated-at.bofh.it> |
On 06/01/2016 01:41 PM, Andrew F. Davis wrote: > Some SoCs contain reset controls for modules that are memory-mapped to > areas shared with other module configuration settings. This requires > synchronization across all drivers accessing this memory area. This > series adds a generic SYSCON reset driver to allow resets toggled > by bits in memory-mapped registers through SYSCON. > > Changes from v2: > - Rebased on v4.7-rc1 > - Removed the need to give reset specifier nodes an index address > > Changes from v1: > - Reset control information is now described in the reset node, this > keeps the reset information centralized for easy verification > - Other small fixups > > Andrew F. Davis (2): > Documentation: dt: reset: Add syscon reset binding > reset: add a SYSCON based reset driver > > .../devicetree/bindings/reset/syscon-reset.txt | 105 ++++++++ > drivers/reset/Kconfig | 10 + > drivers/reset/Makefile | 1 + > drivers/reset/reset-syscon.c | 289 +++++++++++++++++++++ > include/dt-bindings/reset/syscon.h | 23 ++ > 5 files changed, 428 insertions(+) > create mode 100644 Documentation/devicetree/bindings/reset/syscon-reset.txt > create mode 100644 drivers/reset/reset-syscon.c > create mode 100644 include/dt-bindings/reset/syscon.h > Is there any additional feedback for this driver? I normally try to refrain from pings, but this may begin to block further upstreaming of IPs that uses this reset if it can not be taken this cycle. If there is still an objection to calling this a generic reset solution we would not strongly object to relabeling this to something implying more TI exclusivity. Thanks, Andrew
[toc] | [next] | [standalone]
| From | Philipp Zabel <p.zabel@pengutronix.de> |
|---|---|
| Date | 2016-06-15 18:50 +0200 |
| Message-ID | <rKmaT-4Zj-49@gated-at.bofh.it> |
| In reply to | #1423158 |
Hi Andrew,
Am Mittwoch, den 15.06.2016, 10:48 -0500 schrieb Andrew F. Davis:
> On 06/01/2016 01:41 PM, Andrew F. Davis wrote:
> > Some SoCs contain reset controls for modules that are memory-mapped to
> > areas shared with other module configuration settings. This requires
> > synchronization across all drivers accessing this memory area. This
> > series adds a generic SYSCON reset driver to allow resets toggled
> > by bits in memory-mapped registers through SYSCON.
> >
> > Changes from v2:
> > - Rebased on v4.7-rc1
> > - Removed the need to give reset specifier nodes an index address
> >
> > Changes from v1:
> > - Reset control information is now described in the reset node, this
> > keeps the reset information centralized for easy verification
> > - Other small fixups
> >
> > Andrew F. Davis (2):
> > Documentation: dt: reset: Add syscon reset binding
> > reset: add a SYSCON based reset driver
> >
> > .../devicetree/bindings/reset/syscon-reset.txt | 105 ++++++++
> > drivers/reset/Kconfig | 10 +
> > drivers/reset/Makefile | 1 +
> > drivers/reset/reset-syscon.c | 289 +++++++++++++++++++++
> > include/dt-bindings/reset/syscon.h | 23 ++
> > 5 files changed, 428 insertions(+)
> > create mode 100644 Documentation/devicetree/bindings/reset/syscon-reset.txt
> > create mode 100644 drivers/reset/reset-syscon.c
> > create mode 100644 include/dt-bindings/reset/syscon.h
> >
>
> Is there any additional feedback for this driver? I normally try to
> refrain from pings, but this may begin to block further upstreaming of
> IPs that uses this reset if it can not be taken this cycle.
There's still Rob's concern that this binding needs a device tree node
per single register bit in the worst case, which seems a bit overkill.
I'd still prefer to have this information hidden in the drivers, but if
you absolutely have to put it in the device tree, maybe an approach more
similar to pinctrl-simple, where you could list all resets, one per
line, in a single property, would be more acceptable:
pscrst: reset-controller {
compatible = "ti,<chip>-pscrst", "syscon-reset";
/* syscon-reset,bits = <ctrl_reg ctrl_bit stat_reg stat_bit flags>; */
syscon-reset,bits = <0xa3c 8 0x83c 8 RESET_ASSERT_CLEAR /* 0: pcrst-dsp */
0xa40 5 0 0 RESET_TRIGGER_SET>; /* 1: pcrst-example */
};
dsp0: dsp {
resets = <&pscrst 0>;
};
> If there is still an objection to calling this a generic reset solution
> we would not strongly object to relabeling this to something implying
> more TI exclusivity.
Good. Right now there don't seem to be that many reset controllers in
the wild for which this binding would be a good fit. If this turns out
to be useful for others, we can add more compatibles to the driver.
regards
Philipp
[toc] | [prev] | [next] | [standalone]
| From | Rob Herring <robh+dt@kernel.org> |
|---|---|
| Date | 2016-06-16 04:20 +0200 |
| Message-ID | <rKv4u-2mG-13@gated-at.bofh.it> |
| In reply to | #1423230 |
On Wed, Jun 15, 2016 at 11:45 AM, Philipp Zabel <p.zabel@pengutronix.de> wrote:
> Hi Andrew,
>
> Am Mittwoch, den 15.06.2016, 10:48 -0500 schrieb Andrew F. Davis:
>> On 06/01/2016 01:41 PM, Andrew F. Davis wrote:
>> > Some SoCs contain reset controls for modules that are memory-mapped to
>> > areas shared with other module configuration settings. This requires
>> > synchronization across all drivers accessing this memory area. This
>> > series adds a generic SYSCON reset driver to allow resets toggled
>> > by bits in memory-mapped registers through SYSCON.
>> >
>> > Changes from v2:
>> > - Rebased on v4.7-rc1
>> > - Removed the need to give reset specifier nodes an index address
>> >
>> > Changes from v1:
>> > - Reset control information is now described in the reset node, this
>> > keeps the reset information centralized for easy verification
>> > - Other small fixups
>> >
>> > Andrew F. Davis (2):
>> > Documentation: dt: reset: Add syscon reset binding
>> > reset: add a SYSCON based reset driver
>> >
>> > .../devicetree/bindings/reset/syscon-reset.txt | 105 ++++++++
>> > drivers/reset/Kconfig | 10 +
>> > drivers/reset/Makefile | 1 +
>> > drivers/reset/reset-syscon.c | 289 +++++++++++++++++++++
>> > include/dt-bindings/reset/syscon.h | 23 ++
>> > 5 files changed, 428 insertions(+)
>> > create mode 100644 Documentation/devicetree/bindings/reset/syscon-reset.txt
>> > create mode 100644 drivers/reset/reset-syscon.c
>> > create mode 100644 include/dt-bindings/reset/syscon.h
>> >
>>
>> Is there any additional feedback for this driver? I normally try to
>> refrain from pings, but this may begin to block further upstreaming of
>> IPs that uses this reset if it can not be taken this cycle.
>
> There's still Rob's concern that this binding needs a device tree node
> per single register bit in the worst case, which seems a bit overkill.
Yes, that's still my concern with this.
> I'd still prefer to have this information hidden in the drivers, but if
> you absolutely have to put it in the device tree, maybe an approach more
> similar to pinctrl-simple, where you could list all resets, one per
> line, in a single property, would be more acceptable:
>
> pscrst: reset-controller {
> compatible = "ti,<chip>-pscrst", "syscon-reset";
>
> /* syscon-reset,bits = <ctrl_reg ctrl_bit stat_reg stat_bit flags>; */
ti,reset-bits
> syscon-reset,bits = <0xa3c 8 0x83c 8 RESET_ASSERT_CLEAR /* 0: pcrst-dsp */
> 0xa40 5 0 0 RESET_TRIGGER_SET>; /* 1: pcrst-example */
> };
>
> dsp0: dsp {
> resets = <&pscrst 0>;
> };
Otherwise, this seems okay. It is more concise.
>
>> If there is still an objection to calling this a generic reset solution
>> we would not strongly object to relabeling this to something implying
>> more TI exclusivity.
>
> Good. Right now there don't seem to be that many reset controllers in
> the wild for which this binding would be a good fit. If this turns out
> to be useful for others, we can add more compatibles to the driver.
Certainly other users would make a generic solution more compelling.
Rob
[toc] | [prev] | [next] | [standalone]
| From | "Andrew F. Davis" <afd@ti.com> |
|---|---|
| Date | 2016-06-20 21:10 +0200 |
| Message-ID | <rMcK5-3NZ-5@gated-at.bofh.it> |
| In reply to | #1423644 |
On 06/15/2016 09:13 PM, Rob Herring wrote:
> On Wed, Jun 15, 2016 at 11:45 AM, Philipp Zabel <p.zabel@pengutronix.de> wrote:
>> Hi Andrew,
>>
>> Am Mittwoch, den 15.06.2016, 10:48 -0500 schrieb Andrew F. Davis:
>>> On 06/01/2016 01:41 PM, Andrew F. Davis wrote:
>>>> Some SoCs contain reset controls for modules that are memory-mapped to
>>>> areas shared with other module configuration settings. This requires
>>>> synchronization across all drivers accessing this memory area. This
>>>> series adds a generic SYSCON reset driver to allow resets toggled
>>>> by bits in memory-mapped registers through SYSCON.
>>>>
>>>> Changes from v2:
>>>> - Rebased on v4.7-rc1
>>>> - Removed the need to give reset specifier nodes an index address
>>>>
>>>> Changes from v1:
>>>> - Reset control information is now described in the reset node, this
>>>> keeps the reset information centralized for easy verification
>>>> - Other small fixups
>>>>
>>>> Andrew F. Davis (2):
>>>> Documentation: dt: reset: Add syscon reset binding
>>>> reset: add a SYSCON based reset driver
>>>>
>>>> .../devicetree/bindings/reset/syscon-reset.txt | 105 ++++++++
>>>> drivers/reset/Kconfig | 10 +
>>>> drivers/reset/Makefile | 1 +
>>>> drivers/reset/reset-syscon.c | 289 +++++++++++++++++++++
>>>> include/dt-bindings/reset/syscon.h | 23 ++
>>>> 5 files changed, 428 insertions(+)
>>>> create mode 100644 Documentation/devicetree/bindings/reset/syscon-reset.txt
>>>> create mode 100644 drivers/reset/reset-syscon.c
>>>> create mode 100644 include/dt-bindings/reset/syscon.h
>>>>
>>>
>>> Is there any additional feedback for this driver? I normally try to
>>> refrain from pings, but this may begin to block further upstreaming of
>>> IPs that uses this reset if it can not be taken this cycle.
>>
>> There's still Rob's concern that this binding needs a device tree node
>> per single register bit in the worst case, which seems a bit overkill.
>
> Yes, that's still my concern with this.
>
>> I'd still prefer to have this information hidden in the drivers, but if
>> you absolutely have to put it in the device tree, maybe an approach more
>> similar to pinctrl-simple, where you could list all resets, one per
>> line, in a single property, would be more acceptable:
>>
>> pscrst: reset-controller {
>> compatible = "ti,<chip>-pscrst", "syscon-reset";
>>
>> /* syscon-reset,bits = <ctrl_reg ctrl_bit stat_reg stat_bit flags>; */
>
> ti,reset-bits
>
>> syscon-reset,bits = <0xa3c 8 0x83c 8 RESET_ASSERT_CLEAR /* 0: pcrst-dsp */
>> 0xa40 5 0 0 RESET_TRIGGER_SET>; /* 1: pcrst-example */
>> };
>>
>> dsp0: dsp {
>> resets = <&pscrst 0>;
>> };
>
> Otherwise, this seems okay. It is more concise.
>
If this is what you would both be okay with then I'll do it this way,
pushing v4 in a moment.
>>
>>> If there is still an objection to calling this a generic reset solution
>>> we would not strongly object to relabeling this to something implying
>>> more TI exclusivity.
>>
>> Good. Right now there don't seem to be that many reset controllers in
>> the wild for which this binding would be a good fit. If this turns out
>> to be useful for others, we can add more compatibles to the driver.
>
> Certainly other users would make a generic solution more compelling.
>
I believe some other current reset drivers could have made use of this
without having resorted to new drivers, I guess we will have to just
wait to see if it gets traction with new platforms or not, before we can
decide if this solution is really as general as I think it can be.
Andrew
[toc] | [prev] | [standalone]
Back to top | Article view | linux.kernel
csiph-web