diff mbox

[v7,05/15] dt-bindings: Document the STM32 reset bindings

Message ID 1430410844-16062-6-git-send-email-mcoquelin.stm32@gmail.com
State New
Headers show

Commit Message

Maxime Coquelin April 30, 2015, 4:20 p.m. UTC
This adds documentation of device tree bindings for the
STM32 reset controller.

Tested-by: Chanwoo Choi <cw00.choi@samsung.com>
Acked-by: Philipp Zabel <p.zabel@pengutronix.de>
Acked-by: Rob Herring <robh@kernel.org>
Signed-off-by: Maxime Coquelin <mcoquelin.stm32@gmail.com>
---
 .../devicetree/bindings/reset/st,stm32-rcc.txt     | 107 +++++++++++++++++++++
 1 file changed, 107 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/reset/st,stm32-rcc.txt

Comments

Daniel Thompson May 1, 2015, 8:08 a.m. UTC | #1
On 30/04/15 17:20, Maxime Coquelin wrote:
> This adds documentation of device tree bindings for the
> STM32 reset controller.
>
> Tested-by: Chanwoo Choi <cw00.choi@samsung.com>
> Acked-by: Philipp Zabel <p.zabel@pengutronix.de>
> Acked-by: Rob Herring <robh@kernel.org>
> Signed-off-by: Maxime Coquelin <mcoquelin.stm32@gmail.com>
> ---
>   .../devicetree/bindings/reset/st,stm32-rcc.txt     | 107 +++++++++++++++++++++
>   1 file changed, 107 insertions(+)
>   create mode 100644 Documentation/devicetree/bindings/reset/st,stm32-rcc.txt
>
> diff --git a/Documentation/devicetree/bindings/reset/st,stm32-rcc.txt b/Documentation/devicetree/bindings/reset/st,stm32-rcc.txt
> new file mode 100644
> index 0000000..c1b0f8d
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/reset/st,stm32-rcc.txt
> @@ -0,0 +1,107 @@
> +STMicroelectronics STM32 Peripheral Reset Controller
> +====================================================
> +
> +The RCC IP is both a reset and a clock controller. This documentation only
> +documents the reset part.
> +
> +Please also refer to reset.txt in this directory for common reset
> +controller binding usage.
> +
> +Required properties:
> +- compatible: Should be "st,stm32-rcc"
> +- reg: should be register base and length as documented in the
> +  datasheet
> +- #reset-cells: 1, see below
> +
> +example:
> +
> +rcc: reset@40023800 {
> +	#reset-cells = <1>;
> +	compatible = "st,stm32-rcc";

Do you intend the clock driver to use the same compatible string (given 
it is the same bit of hardware).

If so, is it better to use st,stm32f4-rcc here? It seems unlikey to me 
that the register layout of the PLLs and dividers can be the same on the 
f7 parts (and later).

> +	reg = <0x40023800 0x400>;
> +};
> +
> +Specifying softreset control of devices
> +=======================================
> +
> +Device nodes should specify the reset channel required in their "resets"
> +property, containing a phandle to the reset device node and an index specifying
> +which channel to use.
> +The index is the bit number within the RCC registers bank, starting from RCC
> +base address.
> +It is calculated as: index = register_offset / 4 * 32 + bit_offset.
> +Where bit_offset is the bit offset within the register.
> +For example, for CRC reset:
> +  crc = AHB1RSTR_offset / 4 * 32 + CRCRST_bit_offset = 0x10 / 4 * 32 + 12 = 140
> +
> +example:
> +
> +	timer2 {
> +		resets			= <&rcc 256>;
> +	};
> +
> +List of valid indices for STM32F429:
> + - gpioa: 128
> + - gpiob: 129
> + - gpioc: 130
> + - gpiod: 131
> + - gpioe: 132
> + - gpiof: 133
> + - gpiog: 134
> + - gpioh: 135
> + - gpioi: 136
> + - gpioj: 137
> + - gpiok: 138
> + - crc: 140
> + - dma1: 149
> + - dma2: 150
> + - dma2d: 151
> + - ethmac: 153
> + - otghs: 157
> + - dcmi: 160
> + - cryp: 164
> + - hash: 165
> + - rng: 166
> + - otgfs: 167
> + - fmc: 192
> + - tim2: 256
> + - tim3: 257
> + - tim4: 258
> + - tim5: 259
> + - tim6: 260
> + - tim7: 261
> + - tim12: 262
> + - tim13: 263
> + - tim14: 264
> + - wwdg: 267
> + - spi2: 270
> + - spi3: 271
> + - uart2: 273
> + - uart3: 274
> + - uart4: 275
> + - uart5: 276
> + - i2c1: 277
> + - i2c2: 278
> + - i2c3: 279
> + - can1: 281
> + - can2: 282
> + - pwr: 284
> + - dac: 285
> + - uart7: 286
> + - uart8: 287
> + - tim1: 288
> + - tim8: 289
> + - usart1: 292
> + - usart6: 293
> + - adc: 296
> + - sdio: 299
> + - spi1: 300
> + - spi4: 301
> + - syscfg: 302
> + - tim9: 304
> + - tim10: 305
> + - tim11: 306
> + - spi5: 308
> + - spi6: 309
> + - sai1: 310
> + - ltdc: 314

These numbers are stable for all STM32F4 family parts. Should this table 
go into a dt-bindings header file?

--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Maxime Coquelin May 2, 2015, 7:55 a.m. UTC | #2
2015-05-01 10:08 GMT+02:00 Daniel Thompson <daniel.thompson@linaro.org>:
> On 30/04/15 17:20, Maxime Coquelin wrote:
>>
>> This adds documentation of device tree bindings for the
>> STM32 reset controller.
>>
>> Tested-by: Chanwoo Choi <cw00.choi@samsung.com>
>> Acked-by: Philipp Zabel <p.zabel@pengutronix.de>
>> Acked-by: Rob Herring <robh@kernel.org>
>> Signed-off-by: Maxime Coquelin <mcoquelin.stm32@gmail.com>
>> ---
>>   .../devicetree/bindings/reset/st,stm32-rcc.txt     | 107
>> +++++++++++++++++++++
>>   1 file changed, 107 insertions(+)
>>   create mode 100644
>> Documentation/devicetree/bindings/reset/st,stm32-rcc.txt
>>
>> diff --git a/Documentation/devicetree/bindings/reset/st,stm32-rcc.txt
>> b/Documentation/devicetree/bindings/reset/st,stm32-rcc.txt
>> new file mode 100644
>> index 0000000..c1b0f8d
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/reset/st,stm32-rcc.txt
>> @@ -0,0 +1,107 @@
>> +STMicroelectronics STM32 Peripheral Reset Controller
>> +====================================================
>> +
>> +The RCC IP is both a reset and a clock controller. This documentation
>> only
>> +documents the reset part.
>> +
>> +Please also refer to reset.txt in this directory for common reset
>> +controller binding usage.
>> +
>> +Required properties:
>> +- compatible: Should be "st,stm32-rcc"
>> +- reg: should be register base and length as documented in the
>> +  datasheet
>> +- #reset-cells: 1, see below
>> +
>> +example:
>> +
>> +rcc: reset@40023800 {
>> +       #reset-cells = <1>;
>> +       compatible = "st,stm32-rcc";
>
>
> Do you intend the clock driver to use the same compatible string (given it
> is the same bit of hardware).
>
> If so, is it better to use st,stm32f4-rcc here? It seems unlikey to me that
> the register layout of the PLLs and dividers can be the same on the f7 parts
> (and later).

I agree we need a compatible dedicate to f4 series for clocks, and
maybe even one for f429 (to be checked).
For the reset part, we don't have this need.

So either we use only "st,stm32f4" as you suggest, or we can have both
in device tree:

rcc: reset@40023800 {
    #reset-cells = <1>;
    compatible = "st,stm32f4-rcc", "st,stm32-rcc";
    reg = <0x40023800 0x400>;
};

What do you think?

>
>
>> +       reg = <0x40023800 0x400>;
>> +};
>> +
>> +Specifying softreset control of devices
>> +=======================================
>> +
>> +Device nodes should specify the reset channel required in their "resets"
>> +property, containing a phandle to the reset device node and an index
>> specifying
>> +which channel to use.
>> +The index is the bit number within the RCC registers bank, starting from
>> RCC
>> +base address.
>> +It is calculated as: index = register_offset / 4 * 32 + bit_offset.
>> +Where bit_offset is the bit offset within the register.
>> +For example, for CRC reset:
>> +  crc = AHB1RSTR_offset / 4 * 32 + CRCRST_bit_offset = 0x10 / 4 * 32 + 12
>> = 140
>> +
>> +example:
>> +
>> +       timer2 {
>> +               resets                  = <&rcc 256>;
>> +       };
>> +
>> +List of valid indices for STM32F429:
>> + - gpioa: 128
>> + - gpiob: 129
>> + - gpioc: 130
>> + - gpiod: 131
>> + - gpioe: 132
>> + - gpiof: 133
>> + - gpiog: 134
>> + - gpioh: 135
>> + - gpioi: 136
>> + - gpioj: 137
>> + - gpiok: 138
>> + - crc: 140
>> + - dma1: 149
>> + - dma2: 150
>> + - dma2d: 151
>> + - ethmac: 153
>> + - otghs: 157
>> + - dcmi: 160
>> + - cryp: 164
>> + - hash: 165
>> + - rng: 166
>> + - otgfs: 167
>> + - fmc: 192
>> + - tim2: 256
>> + - tim3: 257
>> + - tim4: 258
>> + - tim5: 259
>> + - tim6: 260
>> + - tim7: 261
>> + - tim12: 262
>> + - tim13: 263
>> + - tim14: 264
>> + - wwdg: 267
>> + - spi2: 270
>> + - spi3: 271
>> + - uart2: 273
>> + - uart3: 274
>> + - uart4: 275
>> + - uart5: 276
>> + - i2c1: 277
>> + - i2c2: 278
>> + - i2c3: 279
>> + - can1: 281
>> + - can2: 282
>> + - pwr: 284
>> + - dac: 285
>> + - uart7: 286
>> + - uart8: 287
>> + - tim1: 288
>> + - tim8: 289
>> + - usart1: 292
>> + - usart6: 293
>> + - adc: 296
>> + - sdio: 299
>> + - spi1: 300
>> + - spi4: 301
>> + - syscfg: 302
>> + - tim9: 304
>> + - tim10: 305
>> + - tim11: 306
>> + - spi5: 308
>> + - spi6: 309
>> + - sai1: 310
>> + - ltdc: 314
>
>
> These numbers are stable for all STM32F4 family parts. Should this table go
> into a dt-bindings header file?
>

This has already been discussed with Philipp and Arnd in earlier
versions of this series [0].
I initially created a header file, and we  decided going this way finally.

Regards,
Maxime

[0]: https://lkml.org/lkml/2015/3/10/692
--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Daniel Thompson May 2, 2015, 10:01 a.m. UTC | #3
On 02/05/15 08:55, Maxime Coquelin wrote:
> 2015-05-01 10:08 GMT+02:00 Daniel Thompson <daniel.thompson@linaro.org>:
>> On 30/04/15 17:20, Maxime Coquelin wrote:
>>>
>>> This adds documentation of device tree bindings for the
>>> STM32 reset controller.
>>>
>>> Tested-by: Chanwoo Choi <cw00.choi@samsung.com>
>>> Acked-by: Philipp Zabel <p.zabel@pengutronix.de>
>>> Acked-by: Rob Herring <robh@kernel.org>
>>> Signed-off-by: Maxime Coquelin <mcoquelin.stm32@gmail.com>
>>> ---
>>>    .../devicetree/bindings/reset/st,stm32-rcc.txt     | 107
>>> +++++++++++++++++++++
>>>    1 file changed, 107 insertions(+)
>>>    create mode 100644
>>> Documentation/devicetree/bindings/reset/st,stm32-rcc.txt
>>>
>>> diff --git a/Documentation/devicetree/bindings/reset/st,stm32-rcc.txt
>>> b/Documentation/devicetree/bindings/reset/st,stm32-rcc.txt
>>> new file mode 100644
>>> index 0000000..c1b0f8d
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/reset/st,stm32-rcc.txt
>>> @@ -0,0 +1,107 @@
>>> +STMicroelectronics STM32 Peripheral Reset Controller
>>> +====================================================
>>> +
>>> +The RCC IP is both a reset and a clock controller. This documentation
>>> only
>>> +documents the reset part.
>>> +
>>> +Please also refer to reset.txt in this directory for common reset
>>> +controller binding usage.
>>> +
>>> +Required properties:
>>> +- compatible: Should be "st,stm32-rcc"
>>> +- reg: should be register base and length as documented in the
>>> +  datasheet
>>> +- #reset-cells: 1, see below
>>> +
>>> +example:
>>> +
>>> +rcc: reset@40023800 {
>>> +       #reset-cells = <1>;
>>> +       compatible = "st,stm32-rcc";
>>
>>
>> Do you intend the clock driver to use the same compatible string (given it
>> is the same bit of hardware).
>>
>> If so, is it better to use st,stm32f4-rcc here? It seems unlikey to me that
>> the register layout of the PLLs and dividers can be the same on the f7 parts
>> (and later).
>
> I agree we need a compatible dedicate to f4 series for clocks, and
> maybe even one for f429 (to be checked).
> For the reset part, we don't have this need.
>
> So either we use only "st,stm32f4" as you suggest, or we can have both
> in device tree:
>
> rcc: reset@40023800 {
>      #reset-cells = <1>;
>      compatible = "st,stm32f4-rcc", "st,stm32-rcc";
>      reg = <0x40023800 0x400>;
> };
>
> What do you think?

Having both makes sense. The reset driver probably doesn't care about 
differences between F4 and F7 (I know very little about F7 but I can't 
think of any obvious h/ware evolution that would confuse the current 
reset driver).


>>> +       reg = <0x40023800 0x400>;
>>> +};
>>> +
>>> +Specifying softreset control of devices
>>> +=======================================
>>> +
>>> +Device nodes should specify the reset channel required in their "resets"
>>> +property, containing a phandle to the reset device node and an index
>>> specifying
>>> +which channel to use.
>>> +The index is the bit number within the RCC registers bank, starting from
>>> RCC
>>> +base address.
>>> +It is calculated as: index = register_offset / 4 * 32 + bit_offset.
>>> +Where bit_offset is the bit offset within the register.
>>> +For example, for CRC reset:
>>> +  crc = AHB1RSTR_offset / 4 * 32 + CRCRST_bit_offset = 0x10 / 4 * 32 + 12
>>> = 140
>>> +
>>> +example:
>>> +
>>> +       timer2 {
>>> +               resets                  = <&rcc 256>;
>>> +       };
>>> +
>>> +List of valid indices for STM32F429:
>>> + - gpioa: 128
>>> + - gpiob: 129
>>> ...
>>> <snip>
>>> ...
>>> + - sai1: 310
>>> + - ltdc: 314
>>
>>
>> These numbers are stable for all STM32F4 family parts. Should this table go
>> into a dt-bindings header file?
>>
>
> This has already been discussed with Philipp and Arnd in earlier
> versions of this series [0].
> I initially created a header file, and we  decided going this way finally.

Thanks for the link. I had overlooked that (I only really started paying 
attention at v5; I should probably have looked further back before 
commenting).

However...

Arnd's concerns about mergability of headers can also be met by using 
h/ware values in the header file can't there. To be honest my comment 
was pretty heavily influenced after having read a recent patch from Rob 
Herring ( https://lkml.org/lkml/2015/5/1/14 ) which does exactly this.

The main reason I got interested in having a header is that the reset 
bits and the clock gate bits are encoded using the same bit patterns so 
I wondering it we could express that only once.

I guess it doesn't matter that much, especially given there is only one 
.dtsi file, and we can add a header later and remain binary compatible. 
However if the same number set does end up repeated in different .dtsi 
files I think that would motivate adding a header for F4 family.


Daniel.

--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Philipp Zabel May 4, 2015, 10:28 a.m. UTC | #4
Am Samstag, den 02.05.2015, 11:01 +0100 schrieb Daniel Thompson:
> On 02/05/15 08:55, Maxime Coquelin wrote:
> > 2015-05-01 10:08 GMT+02:00 Daniel Thompson <daniel.thompson@linaro.org>:
[...]
> >> Do you intend the clock driver to use the same compatible string (given it
> >> is the same bit of hardware).
> >>
> >> If so, is it better to use st,stm32f4-rcc here? It seems unlikey to me that
> >> the register layout of the PLLs and dividers can be the same on the f7 parts
> >> (and later).
> >
> > I agree we need a compatible dedicate to f4 series for clocks, and
> > maybe even one for f429 (to be checked).
> > For the reset part, we don't have this need.
> >
> > So either we use only "st,stm32f4" as you suggest, or we can have both
> > in device tree:
> >
> > rcc: reset@40023800 {
> >      #reset-cells = <1>;
> >      compatible = "st,stm32f4-rcc", "st,stm32-rcc";
> >      reg = <0x40023800 0x400>;
> > };
> >
> > What do you think?
> 
> Having both makes sense. The reset driver probably doesn't care about 
> differences between F4 and F7 (I know very little about F7 but I can't 
> think of any obvious h/ware evolution that would confuse the current 
> reset driver).

Seconded, this is exactly the way compatible string lists are supposed
to be used.

[...]

regards
Philipp

--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Maxime Coquelin May 4, 2015, 11:11 a.m. UTC | #5
2015-05-04 12:28 GMT+02:00 Philipp Zabel <p.zabel@pengutronix.de>:
> Am Samstag, den 02.05.2015, 11:01 +0100 schrieb Daniel Thompson:
>> On 02/05/15 08:55, Maxime Coquelin wrote:
>> > 2015-05-01 10:08 GMT+02:00 Daniel Thompson <daniel.thompson@linaro.org>:
> [...]
>> >> Do you intend the clock driver to use the same compatible string (given it
>> >> is the same bit of hardware).
>> >>
>> >> If so, is it better to use st,stm32f4-rcc here? It seems unlikey to me that
>> >> the register layout of the PLLs and dividers can be the same on the f7 parts
>> >> (and later).
>> >
>> > I agree we need a compatible dedicate to f4 series for clocks, and
>> > maybe even one for f429 (to be checked).
>> > For the reset part, we don't have this need.
>> >
>> > So either we use only "st,stm32f4" as you suggest, or we can have both
>> > in device tree:
>> >
>> > rcc: reset@40023800 {
>> >      #reset-cells = <1>;
>> >      compatible = "st,stm32f4-rcc", "st,stm32-rcc";
>> >      reg = <0x40023800 0x400>;
>> > };
>> >
>> > What do you think?
>>
>> Having both makes sense. The reset driver probably doesn't care about
>> differences between F4 and F7 (I know very little about F7 but I can't
>> think of any obvious h/ware evolution that would confuse the current
>> reset driver).
>
> Seconded, this is exactly the way compatible string lists are supposed
> to be used.

Ok good, so we all agree.

I propose I keep "st,stm32-rcc" only for this series, as it does not
contain the clock driver.
The series adding the clock driver will add "st,stm32f4-rcc", or
"st,stm32f429-rcc" depending on clock driver needs.

Thanks,
Maxime

>
> [...]
>
> regards
> Philipp
>
--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Maxime Coquelin May 4, 2015, 11:25 a.m. UTC | #6
2015-05-02 12:01 GMT+02:00 Daniel Thompson <daniel.thompson@linaro.org>:
> On 02/05/15 08:55, Maxime Coquelin wrote:
>>
>> 2015-05-01 10:08 GMT+02:00 Daniel Thompson <daniel.thompson@linaro.org>:
>>>
>>> On 30/04/15 17:20, Maxime Coquelin wrote:
>>>>
>>>>
>>>> This adds documentation of device tree bindings for the
>>>> STM32 reset controller.
>>>>
>>>> Tested-by: Chanwoo Choi <cw00.choi@samsung.com>
>>>> Acked-by: Philipp Zabel <p.zabel@pengutronix.de>
>>>> Acked-by: Rob Herring <robh@kernel.org>
>>>> Signed-off-by: Maxime Coquelin <mcoquelin.stm32@gmail.com>
>>>> ---
>>>>    .../devicetree/bindings/reset/st,stm32-rcc.txt     | 107
>>>> +++++++++++++++++++++
>>>>    1 file changed, 107 insertions(+)
>>>>    create mode 100644
>>>> Documentation/devicetree/bindings/reset/st,stm32-rcc.txt
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/reset/st,stm32-rcc.txt
>>>> b/Documentation/devicetree/bindings/reset/st,stm32-rcc.txt
>>>> new file mode 100644
>>>> index 0000000..c1b0f8d
>>>> --- /dev/null
>>>> +++ b/Documentation/devicetree/bindings/reset/st,stm32-rcc.txt
>>>> @@ -0,0 +1,107 @@
>>>> +STMicroelectronics STM32 Peripheral Reset Controller
>>>> +====================================================
>>>> +
>>>> +The RCC IP is both a reset and a clock controller. This documentation
>>>> only
>>>> +documents the reset part.
>>>> +
>>>> +Please also refer to reset.txt in this directory for common reset
>>>> +controller binding usage.
>>>> +
>>>> +Required properties:
>>>> +- compatible: Should be "st,stm32-rcc"
>>>> +- reg: should be register base and length as documented in the
>>>> +  datasheet
>>>> +- #reset-cells: 1, see below
>>>> +
>>>> +example:
>>>> +
>>>> +rcc: reset@40023800 {
>>>> +       #reset-cells = <1>;
>>>> +       compatible = "st,stm32-rcc";
>>>
>>>
>>>
>>> Do you intend the clock driver to use the same compatible string (given
>>> it
>>> is the same bit of hardware).
>>>
>>> If so, is it better to use st,stm32f4-rcc here? It seems unlikey to me
>>> that
>>> the register layout of the PLLs and dividers can be the same on the f7
>>> parts
>>> (and later).
>>
>>
>> I agree we need a compatible dedicate to f4 series for clocks, and
>> maybe even one for f429 (to be checked).
>> For the reset part, we don't have this need.
>>
>> So either we use only "st,stm32f4" as you suggest, or we can have both
>> in device tree:
>>
>> rcc: reset@40023800 {
>>      #reset-cells = <1>;
>>      compatible = "st,stm32f4-rcc", "st,stm32-rcc";
>>      reg = <0x40023800 0x400>;
>> };
>>
>> What do you think?
>
>
> Having both makes sense. The reset driver probably doesn't care about
> differences between F4 and F7 (I know very little about F7 but I can't think
> of any obvious h/ware evolution that would confuse the current reset
> driver).
>
>
>>>> +       reg = <0x40023800 0x400>;
>>>> +};
>>>> +
>>>> +Specifying softreset control of devices
>>>> +=======================================
>>>> +
>>>> +Device nodes should specify the reset channel required in their
>>>> "resets"
>>>> +property, containing a phandle to the reset device node and an index
>>>> specifying
>>>> +which channel to use.
>>>> +The index is the bit number within the RCC registers bank, starting
>>>> from
>>>> RCC
>>>> +base address.
>>>> +It is calculated as: index = register_offset / 4 * 32 + bit_offset.
>>>> +Where bit_offset is the bit offset within the register.
>>>> +For example, for CRC reset:
>>>> +  crc = AHB1RSTR_offset / 4 * 32 + CRCRST_bit_offset = 0x10 / 4 * 32 +
>>>> 12
>>>> = 140
>>>> +
>>>> +example:
>>>> +
>>>> +       timer2 {
>>>> +               resets                  = <&rcc 256>;
>>>> +       };
>>>> +
>>>> +List of valid indices for STM32F429:
>>>> + - gpioa: 128
>>>> + - gpiob: 129
>>>> ...
>>>> <snip>
>>>> ...
>>>> + - sai1: 310
>>>> + - ltdc: 314
>>>
>>>
>>>
>>> These numbers are stable for all STM32F4 family parts. Should this table
>>> go
>>> into a dt-bindings header file?
>>>
>>
>> This has already been discussed with Philipp and Arnd in earlier
>> versions of this series [0].
>> I initially created a header file, and we  decided going this way finally.
>
>
> Thanks for the link. I had overlooked that (I only really started paying
> attention at v5; I should probably have looked further back before
> commenting).
>
> However...
>
> Arnd's concerns about mergability of headers can also be met by using h/ware
> values in the header file can't there. To be honest my comment was pretty
> heavily influenced after having read a recent patch from Rob Herring (
> https://lkml.org/lkml/2015/5/1/14 ) which does exactly this.
>
> The main reason I got interested in having a header is that the reset bits
> and the clock gate bits are encoded using the same bit patterns so I
> wondering it we could express that only once.

Ok, I understand your need, and it makes sense.
The problem is that the values defined today cannot be re-used
directly for clocks.
Since the calculation is starting from rcc base, there is a 128 offset.

To re-use the same values, maybe we should create a mfd dt-binding header file.
It would contain the bits definition starting from 0, and  define
macros for both reset and clocks to add the offsets.

For example, includes/dt-bindings/mfd/stm32f4-rcc.h would look like:

#define GPIOA 0
#define GPIOB 1
...
#define LTDC 186

#define STM32F4_RESET(x) (x + 128)
#define STM32F4_CLOCK(x) (x + 384)

Then, in DT, a reset would be described like this:

timer2 {
    resets = <&rcc STM32F4_RESET(TIM2)>;
};

Phillip, Daniel, does that look acceptable to you?

Kind regards,
Maxime
>
> I guess it doesn't matter that much, especially given there is only one
> .dtsi file, and we can add a header later and remain binary compatible.
> However if the same number set does end up repeated in different .dtsi files
> I think that would motivate adding a header for F4 family.
>
>
> Daniel.
>
--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Daniel Thompson May 5, 2015, 2:07 p.m. UTC | #7
On 04/05/15 12:25, Maxime Coquelin wrote:
> 2015-05-02 12:01 GMT+02:00 Daniel Thompson <daniel.thompson@linaro.org>:
>> On 02/05/15 08:55, Maxime Coquelin wrote:
>>>
>>> 2015-05-01 10:08 GMT+02:00 Daniel Thompson <daniel.thompson@linaro.org>:
>>>>
>>>> On 30/04/15 17:20, Maxime Coquelin wrote:
>>>>>
>>>>>
>>>>> This adds documentation of device tree bindings for the
>>>>> STM32 reset controller.
>>>>>
>>>>> Tested-by: Chanwoo Choi <cw00.choi@samsung.com>
>>>>> Acked-by: Philipp Zabel <p.zabel@pengutronix.de>
>>>>> Acked-by: Rob Herring <robh@kernel.org>
>>>>> Signed-off-by: Maxime Coquelin <mcoquelin.stm32@gmail.com>
>>>>> ---
>>>>>     .../devicetree/bindings/reset/st,stm32-rcc.txt     | 107
>>>>> +++++++++++++++++++++
>>>>>     1 file changed, 107 insertions(+)
>>>>>     create mode 100644
>>>>> Documentation/devicetree/bindings/reset/st,stm32-rcc.txt
>>>>>
>>>>> diff --git a/Documentation/devicetree/bindings/reset/st,stm32-rcc.txt
>>>>> b/Documentation/devicetree/bindings/reset/st,stm32-rcc.txt
>>>>> new file mode 100644
>>>>> index 0000000..c1b0f8d
>>>>> --- /dev/null
>>>>> +++ b/Documentation/devicetree/bindings/reset/st,stm32-rcc.txt
>>>>> @@ -0,0 +1,107 @@
>>>>> +STMicroelectronics STM32 Peripheral Reset Controller
>>>>> +====================================================
>>>>> +
>>>>> +The RCC IP is both a reset and a clock controller. This documentation
>>>>> only
>>>>> +documents the reset part.
>>>>> +
>>>>> +Please also refer to reset.txt in this directory for common reset
>>>>> +controller binding usage.
>>>>> +
>>>>> +Required properties:
>>>>> +- compatible: Should be "st,stm32-rcc"
>>>>> +- reg: should be register base and length as documented in the
>>>>> +  datasheet
>>>>> +- #reset-cells: 1, see below
>>>>> +
>>>>> +example:
>>>>> +
>>>>> +rcc: reset@40023800 {
>>>>> +       #reset-cells = <1>;
>>>>> +       compatible = "st,stm32-rcc";
>>>>
>>>>
>>>>
>>>> Do you intend the clock driver to use the same compatible string (given
>>>> it
>>>> is the same bit of hardware).
>>>>
>>>> If so, is it better to use st,stm32f4-rcc here? It seems unlikey to me
>>>> that
>>>> the register layout of the PLLs and dividers can be the same on the f7
>>>> parts
>>>> (and later).
>>>
>>>
>>> I agree we need a compatible dedicate to f4 series for clocks, and
>>> maybe even one for f429 (to be checked).
>>> For the reset part, we don't have this need.
>>>
>>> So either we use only "st,stm32f4" as you suggest, or we can have both
>>> in device tree:
>>>
>>> rcc: reset@40023800 {
>>>       #reset-cells = <1>;
>>>       compatible = "st,stm32f4-rcc", "st,stm32-rcc";
>>>       reg = <0x40023800 0x400>;
>>> };
>>>
>>> What do you think?
>>
>>
>> Having both makes sense. The reset driver probably doesn't care about
>> differences between F4 and F7 (I know very little about F7 but I can't think
>> of any obvious h/ware evolution that would confuse the current reset
>> driver).
>>
>>
>>>>> +       reg = <0x40023800 0x400>;
>>>>> +};
>>>>> +
>>>>> +Specifying softreset control of devices
>>>>> +=======================================
>>>>> +
>>>>> +Device nodes should specify the reset channel required in their
>>>>> "resets"
>>>>> +property, containing a phandle to the reset device node and an index
>>>>> specifying
>>>>> +which channel to use.
>>>>> +The index is the bit number within the RCC registers bank, starting
>>>>> from
>>>>> RCC
>>>>> +base address.
>>>>> +It is calculated as: index = register_offset / 4 * 32 + bit_offset.
>>>>> +Where bit_offset is the bit offset within the register.
>>>>> +For example, for CRC reset:
>>>>> +  crc = AHB1RSTR_offset / 4 * 32 + CRCRST_bit_offset = 0x10 / 4 * 32 +
>>>>> 12
>>>>> = 140
>>>>> +
>>>>> +example:
>>>>> +
>>>>> +       timer2 {
>>>>> +               resets                  = <&rcc 256>;
>>>>> +       };
>>>>> +
>>>>> +List of valid indices for STM32F429:
>>>>> + - gpioa: 128
>>>>> + - gpiob: 129
>>>>> ...
>>>>> <snip>
>>>>> ...
>>>>> + - sai1: 310
>>>>> + - ltdc: 314
>>>>
>>>>
>>>>
>>>> These numbers are stable for all STM32F4 family parts. Should this table
>>>> go
>>>> into a dt-bindings header file?
>>>>
>>>
>>> This has already been discussed with Philipp and Arnd in earlier
>>> versions of this series [0].
>>> I initially created a header file, and we  decided going this way finally.
>>
>>
>> Thanks for the link. I had overlooked that (I only really started paying
>> attention at v5; I should probably have looked further back before
>> commenting).
>>
>> However...
>>
>> Arnd's concerns about mergability of headers can also be met by using h/ware
>> values in the header file can't there. To be honest my comment was pretty
>> heavily influenced after having read a recent patch from Rob Herring (
>> https://lkml.org/lkml/2015/5/1/14 ) which does exactly this.
>>
>> The main reason I got interested in having a header is that the reset bits
>> and the clock gate bits are encoded using the same bit patterns so I
>> wondering it we could express that only once.
>
> Ok, I understand your need, and it makes sense.
> The problem is that the values defined today cannot be re-used
> directly for clocks.
> Since the calculation is starting from rcc base, there is a 128 offset.
>
> To re-use the same values, maybe we should create a mfd dt-binding header file.
> It would contain the bits definition starting from 0, and  define
> macros for both reset and clocks to add the offsets.
>
> For example, includes/dt-bindings/mfd/stm32f4-rcc.h would look like:
>
> #define GPIOA 0
> #define GPIOB 1
> ...
> #define LTDC 186
>
> #define STM32F4_RESET(x) (x + 128)
> #define STM32F4_CLOCK(x) (x + 384)
>
> Then, in DT, a reset would be described like this:
>
> timer2 {
>      resets = <&rcc STM32F4_RESET(TIM2)>;
> };
>
> Phillip, Daniel, does that look acceptable to you?

Doesn't look unreasonable.

I am a little uneasy simply because there are very few similar header 
files in that directory but I haven't thought of a better idea.


Daniel.


>
> Kind regards,
> Maxime
>>
>> I guess it doesn't matter that much, especially given there is only one
>> .dtsi file, and we can add a header later and remain binary compatible.
>> However if the same number set does end up repeated in different .dtsi files
>> I think that would motivate adding a header for F4 family.
>>
>>
>> Daniel.
>>

--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Maxime Coquelin May 5, 2015, 3:19 p.m. UTC | #8
2015-05-05 16:07 GMT+02:00 Daniel Thompson <daniel.thompson@linaro.org>:
> On 04/05/15 12:25, Maxime Coquelin wrote:
>>
>> 2015-05-02 12:01 GMT+02:00 Daniel Thompson <daniel.thompson@linaro.org>:
>>>
>>> On 02/05/15 08:55, Maxime Coquelin wrote:
>>>>
>>>>
>>>> 2015-05-01 10:08 GMT+02:00 Daniel Thompson <daniel.thompson@linaro.org>:
>>>>>
>>>>>
>>>>> On 30/04/15 17:20, Maxime Coquelin wrote:
>>>>>>
>>>>>>
>>>>>>
>>>>>> This adds documentation of device tree bindings for the
>>>>>> STM32 reset controller.
>>>>>>
>>>>>> Tested-by: Chanwoo Choi <cw00.choi@samsung.com>
>>>>>> Acked-by: Philipp Zabel <p.zabel@pengutronix.de>
>>>>>> Acked-by: Rob Herring <robh@kernel.org>
>>>>>> Signed-off-by: Maxime Coquelin <mcoquelin.stm32@gmail.com>
>>>>>> ---
>>>>>>     .../devicetree/bindings/reset/st,stm32-rcc.txt     | 107
>>>>>> +++++++++++++++++++++
>>>>>>     1 file changed, 107 insertions(+)
>>>>>>     create mode 100644
>>>>>> Documentation/devicetree/bindings/reset/st,stm32-rcc.txt
>>>>>>
>>>>>> diff --git a/Documentation/devicetree/bindings/reset/st,stm32-rcc.txt
>>>>>> b/Documentation/devicetree/bindings/reset/st,stm32-rcc.txt
>>>>>> new file mode 100644
>>>>>> index 0000000..c1b0f8d
>>>>>> --- /dev/null
>>>>>> +++ b/Documentation/devicetree/bindings/reset/st,stm32-rcc.txt
>>>>>> @@ -0,0 +1,107 @@
>>>>>> +STMicroelectronics STM32 Peripheral Reset Controller
>>>>>> +====================================================
>>>>>> +
>>>>>> +The RCC IP is both a reset and a clock controller. This documentation
>>>>>> only
>>>>>> +documents the reset part.
>>>>>> +
>>>>>> +Please also refer to reset.txt in this directory for common reset
>>>>>> +controller binding usage.
>>>>>> +
>>>>>> +Required properties:
>>>>>> +- compatible: Should be "st,stm32-rcc"
>>>>>> +- reg: should be register base and length as documented in the
>>>>>> +  datasheet
>>>>>> +- #reset-cells: 1, see below
>>>>>> +
>>>>>> +example:
>>>>>> +
>>>>>> +rcc: reset@40023800 {
>>>>>> +       #reset-cells = <1>;
>>>>>> +       compatible = "st,stm32-rcc";
>>>>>
>>>>>
>>>>>
>>>>>
>>>>> Do you intend the clock driver to use the same compatible string (given
>>>>> it
>>>>> is the same bit of hardware).
>>>>>
>>>>> If so, is it better to use st,stm32f4-rcc here? It seems unlikey to me
>>>>> that
>>>>> the register layout of the PLLs and dividers can be the same on the f7
>>>>> parts
>>>>> (and later).
>>>>
>>>>
>>>>
>>>> I agree we need a compatible dedicate to f4 series for clocks, and
>>>> maybe even one for f429 (to be checked).
>>>> For the reset part, we don't have this need.
>>>>
>>>> So either we use only "st,stm32f4" as you suggest, or we can have both
>>>> in device tree:
>>>>
>>>> rcc: reset@40023800 {
>>>>       #reset-cells = <1>;
>>>>       compatible = "st,stm32f4-rcc", "st,stm32-rcc";
>>>>       reg = <0x40023800 0x400>;
>>>> };
>>>>
>>>> What do you think?
>>>
>>>
>>>
>>> Having both makes sense. The reset driver probably doesn't care about
>>> differences between F4 and F7 (I know very little about F7 but I can't
>>> think
>>> of any obvious h/ware evolution that would confuse the current reset
>>> driver).
>>>
>>>
>>>>>> +       reg = <0x40023800 0x400>;
>>>>>> +};
>>>>>> +
>>>>>> +Specifying softreset control of devices
>>>>>> +=======================================
>>>>>> +
>>>>>> +Device nodes should specify the reset channel required in their
>>>>>> "resets"
>>>>>> +property, containing a phandle to the reset device node and an index
>>>>>> specifying
>>>>>> +which channel to use.
>>>>>> +The index is the bit number within the RCC registers bank, starting
>>>>>> from
>>>>>> RCC
>>>>>> +base address.
>>>>>> +It is calculated as: index = register_offset / 4 * 32 + bit_offset.
>>>>>> +Where bit_offset is the bit offset within the register.
>>>>>> +For example, for CRC reset:
>>>>>> +  crc = AHB1RSTR_offset / 4 * 32 + CRCRST_bit_offset = 0x10 / 4 * 32
>>>>>> +
>>>>>> 12
>>>>>> = 140
>>>>>> +
>>>>>> +example:
>>>>>> +
>>>>>> +       timer2 {
>>>>>> +               resets                  = <&rcc 256>;
>>>>>> +       };
>>>>>> +
>>>>>> +List of valid indices for STM32F429:
>>>>>> + - gpioa: 128
>>>>>> + - gpiob: 129
>>>>>> ...
>>>>>> <snip>
>>>>>> ...
>>>>>> + - sai1: 310
>>>>>> + - ltdc: 314
>>>>>
>>>>>
>>>>>
>>>>>
>>>>> These numbers are stable for all STM32F4 family parts. Should this
>>>>> table
>>>>> go
>>>>> into a dt-bindings header file?
>>>>>
>>>>
>>>> This has already been discussed with Philipp and Arnd in earlier
>>>> versions of this series [0].
>>>> I initially created a header file, and we  decided going this way
>>>> finally.
>>>
>>>
>>>
>>> Thanks for the link. I had overlooked that (I only really started paying
>>> attention at v5; I should probably have looked further back before
>>> commenting).
>>>
>>> However...
>>>
>>> Arnd's concerns about mergability of headers can also be met by using
>>> h/ware
>>> values in the header file can't there. To be honest my comment was pretty
>>> heavily influenced after having read a recent patch from Rob Herring (
>>> https://lkml.org/lkml/2015/5/1/14 ) which does exactly this.
>>>
>>> The main reason I got interested in having a header is that the reset
>>> bits
>>> and the clock gate bits are encoded using the same bit patterns so I
>>> wondering it we could express that only once.
>>
>>
>> Ok, I understand your need, and it makes sense.
>> The problem is that the values defined today cannot be re-used
>> directly for clocks.
>> Since the calculation is starting from rcc base, there is a 128 offset.
>>
>> To re-use the same values, maybe we should create a mfd dt-binding header
>> file.
>> It would contain the bits definition starting from 0, and  define
>> macros for both reset and clocks to add the offsets.
>>
>> For example, includes/dt-bindings/mfd/stm32f4-rcc.h would look like:
>>
>> #define GPIOA 0
>> #define GPIOB 1
>> ...
>> #define LTDC 186
>>
>> #define STM32F4_RESET(x) (x + 128)
>> #define STM32F4_CLOCK(x) (x + 384)
>>
>> Then, in DT, a reset would be described like this:
>>
>> timer2 {
>>      resets = <&rcc STM32F4_RESET(TIM2)>;
>> };
>>
>> Phillip, Daniel, does that look acceptable to you?
>
>
> Doesn't look unreasonable.
>
> I am a little uneasy simply because there are very few similar header files
> in that directory but I haven't thought of a better idea.

Since this file will be shared by both clock and reset drivers, I
don't see better option.
I will implement it in v8 if Philipp agrees.

Thanks,
Maxime

>
>
> Daniel.
>
>
>
>>
>> Kind regards,
>> Maxime
>>>
>>>
>>> I guess it doesn't matter that much, especially given there is only one
>>> .dtsi file, and we can add a header later and remain binary compatible.
>>> However if the same number set does end up repeated in different .dtsi
>>> files
>>> I think that would motivate adding a header for F4 family.
>>>
>>>
>>> Daniel.
>>>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Philipp Zabel May 5, 2015, 3:42 p.m. UTC | #9
Am Dienstag, den 05.05.2015, 17:19 +0200 schrieb Maxime Coquelin:
> >> For example, includes/dt-bindings/mfd/stm32f4-rcc.h would look like:
> >>
> >> #define GPIOA 0
> >> #define GPIOB 1
> >> ...
> >> #define LTDC 186

That looks a bit fragile.
At least the defines for the indices should be properly namespaced,
check out include/dt-bindings/gpio/tegra-gpio.h for a similar case.

> >> #define STM32F4_RESET(x) (x + 128)
> >> #define STM32F4_CLOCK(x) (x + 384)
> >>
> >> Then, in DT, a reset would be described like this:
> >>
> >> timer2 {
> >>      resets = <&rcc STM32F4_RESET(TIM2)>;
> >> };
> >>
> >> Phillip, Daniel, does that look acceptable to you?
> >
> >
> > Doesn't look unreasonable.
> >
> > I am a little uneasy simply because there are very few similar header files
> > in that directory but I haven't thought of a better idea.
> 
> Since this file will be shared by both clock and reset drivers, I
> don't see better option.
> I will implement it in v8 if Philipp agrees.

Are the device tree maintainers happy with this idiom spreading?
Except for the point above, I think this is acceptable.

regards
Philipp

--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Daniel Thompson May 5, 2015, 4:07 p.m. UTC | #10
On 05/05/15 16:42, Philipp Zabel wrote:
> Am Dienstag, den 05.05.2015, 17:19 +0200 schrieb Maxime Coquelin:
>>>> For example, includes/dt-bindings/mfd/stm32f4-rcc.h would look like:
>>>>
>>>> #define GPIOA 0
>>>> #define GPIOB 1
>>>> ...
>>>> #define LTDC 186
>
> That looks a bit fragile.
> At least the defines for the indices should be properly namespaced,
> check out include/dt-bindings/gpio/tegra-gpio.h for a similar case.

Good point.

>>>> #define STM32F4_RESET(x) (x + 128)
>>>> #define STM32F4_CLOCK(x) (x + 384)

Thinking more about this point, if we are going to follow hardware if 
might be better to have:

#define STM32F4_RCC_AHB1_GPIOA 0
#define STM32F4_RCC_AHB1_GPIOA 1
...
#define STM32F4_RCC_APB2_LTDC 26


#define STM32F4_AHB1_RESET(x) (STM32F4_RCC_AHB1_##x##_BIT + (0x10 * 8))
#define STM32F4_AHB2_RESET(x) (STM32F4_RCC_AHB2_##x##_BIT + (0x14 * 8))
...
#define STM32F4_APB2_RESET(x) (STM32F4_RCC_APB2_##x##_BIT + (0x24 * 8))

Its more typing (or copy 'n pasting) by at least every number now maps 
directly to the datasheet.


>>>>
>>>> Then, in DT, a reset would be described like this:
>>>>
>>>> timer2 {
>>>>       resets = <&rcc STM32F4_RESET(TIM2)>;
>>>> };
>>>>
>>>> Phillip, Daniel, does that look acceptable to you?
>>>
>>>
>>> Doesn't look unreasonable.
>>>
>>> I am a little uneasy simply because there are very few similar header files
>>> in that directory but I haven't thought of a better idea.
>>
>> Since this file will be shared by both clock and reset drivers, I
>> don't see better option.
>> I will implement it in v8 if Philipp agrees.
>
> Are the device tree maintainers happy with this idiom spreading?
> Except for the point above, I think this is acceptable.
>
> regards
> Philipp
>

--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Maxime Coquelin May 5, 2015, 5:22 p.m. UTC | #11
2015-05-05 17:42 GMT+02:00 Philipp Zabel <p.zabel@pengutronix.de>:
> Am Dienstag, den 05.05.2015, 17:19 +0200 schrieb Maxime Coquelin:
>> >> For example, includes/dt-bindings/mfd/stm32f4-rcc.h would look like:
>> >>
>> >> #define GPIOA 0
>> >> #define GPIOB 1
>> >> ...
>> >> #define LTDC 186
>
> That looks a bit fragile.
> At least the defines for the indices should be properly namespaced,
> check out include/dt-bindings/gpio/tegra-gpio.h for a similar case.

Thanks, I will prefix them with the proper namespace, Daniel proposal
is fine to me.

>
>> >> #define STM32F4_RESET(x) (x + 128)
>> >> #define STM32F4_CLOCK(x) (x + 384)
>> >>
>> >> Then, in DT, a reset would be described like this:
>> >>
>> >> timer2 {
>> >>      resets = <&rcc STM32F4_RESET(TIM2)>;
>> >> };
>> >>
>> >> Phillip, Daniel, does that look acceptable to you?
>> >
>> >
>> > Doesn't look unreasonable.
>> >
>> > I am a little uneasy simply because there are very few similar header files
>> > in that directory but I haven't thought of a better idea.
>>
>> Since this file will be shared by both clock and reset drivers, I
>> don't see better option.
>> I will implement it in v8 if Philipp agrees.
>
> Are the device tree maintainers happy with this idiom spreading?
> Except for the point above, I think this is acceptable.
Ok good. Let's see what DT maintainers thinks about that.

Regards,
Maxime

>
> regards
> Philipp
>
--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Maxime Coquelin May 5, 2015, 5:24 p.m. UTC | #12
2015-05-05 18:07 GMT+02:00 Daniel Thompson <daniel.thompson@linaro.org>:
> On 05/05/15 16:42, Philipp Zabel wrote:
>>
>> Am Dienstag, den 05.05.2015, 17:19 +0200 schrieb Maxime Coquelin:
>>>>>
>>>>> For example, includes/dt-bindings/mfd/stm32f4-rcc.h would look like:
>>>>>
>>>>> #define GPIOA 0
>>>>> #define GPIOB 1
>>>>> ...
>>>>> #define LTDC 186
>>
>>
>> That looks a bit fragile.
>> At least the defines for the indices should be properly namespaced,
>> check out include/dt-bindings/gpio/tegra-gpio.h for a similar case.
>
>
> Good point.
>
>>>>> #define STM32F4_RESET(x) (x + 128)
>>>>> #define STM32F4_CLOCK(x) (x + 384)
>
>
> Thinking more about this point, if we are going to follow hardware if might
> be better to have:
>
> #define STM32F4_RCC_AHB1_GPIOA 0
> #define STM32F4_RCC_AHB1_GPIOA 1
> ...
> #define STM32F4_RCC_APB2_LTDC 26
>
>
> #define STM32F4_AHB1_RESET(x) (STM32F4_RCC_AHB1_##x##_BIT + (0x10 * 8))
> #define STM32F4_AHB2_RESET(x) (STM32F4_RCC_AHB2_##x##_BIT + (0x14 * 8))
> ...
> #define STM32F4_APB2_RESET(x) (STM32F4_RCC_APB2_##x##_BIT + (0x24 * 8))
>
> Its more typing (or copy 'n pasting) by at least every number now maps
> directly to the datasheet.

As said in Philipp's reply, I like the idea.

Regards,
Maxime
>
>
>
>>>>>
>>>>> Then, in DT, a reset would be described like this:
>>>>>
>>>>> timer2 {
>>>>>       resets = <&rcc STM32F4_RESET(TIM2)>;
>>>>> };
>>>>>
>>>>> Phillip, Daniel, does that look acceptable to you?
>>>>
>>>>
>>>>
>>>> Doesn't look unreasonable.
>>>>
>>>> I am a little uneasy simply because there are very few similar header
>>>> files
>>>> in that directory but I haven't thought of a better idea.
>>>
>>>
>>> Since this file will be shared by both clock and reset drivers, I
>>> don't see better option.
>>> I will implement it in v8 if Philipp agrees.
>>
>>
>> Are the device tree maintainers happy with this idiom spreading?
>> Except for the point above, I think this is acceptable.
>>
>> regards
>> Philipp
>>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/Documentation/devicetree/bindings/reset/st,stm32-rcc.txt b/Documentation/devicetree/bindings/reset/st,stm32-rcc.txt
new file mode 100644
index 0000000..c1b0f8d
--- /dev/null
+++ b/Documentation/devicetree/bindings/reset/st,stm32-rcc.txt
@@ -0,0 +1,107 @@ 
+STMicroelectronics STM32 Peripheral Reset Controller
+====================================================
+
+The RCC IP is both a reset and a clock controller. This documentation only
+documents the reset part.
+
+Please also refer to reset.txt in this directory for common reset
+controller binding usage.
+
+Required properties:
+- compatible: Should be "st,stm32-rcc"
+- reg: should be register base and length as documented in the
+  datasheet
+- #reset-cells: 1, see below
+
+example:
+
+rcc: reset@40023800 {
+	#reset-cells = <1>;
+	compatible = "st,stm32-rcc";
+	reg = <0x40023800 0x400>;
+};
+
+Specifying softreset control of devices
+=======================================
+
+Device nodes should specify the reset channel required in their "resets"
+property, containing a phandle to the reset device node and an index specifying
+which channel to use.
+The index is the bit number within the RCC registers bank, starting from RCC
+base address.
+It is calculated as: index = register_offset / 4 * 32 + bit_offset.
+Where bit_offset is the bit offset within the register.
+For example, for CRC reset:
+  crc = AHB1RSTR_offset / 4 * 32 + CRCRST_bit_offset = 0x10 / 4 * 32 + 12 = 140
+
+example:
+
+	timer2 {
+		resets			= <&rcc 256>;
+	};
+
+List of valid indices for STM32F429:
+ - gpioa: 128
+ - gpiob: 129
+ - gpioc: 130
+ - gpiod: 131
+ - gpioe: 132
+ - gpiof: 133
+ - gpiog: 134
+ - gpioh: 135
+ - gpioi: 136
+ - gpioj: 137
+ - gpiok: 138
+ - crc: 140
+ - dma1: 149
+ - dma2: 150
+ - dma2d: 151
+ - ethmac: 153
+ - otghs: 157
+ - dcmi: 160
+ - cryp: 164
+ - hash: 165
+ - rng: 166
+ - otgfs: 167
+ - fmc: 192
+ - tim2: 256
+ - tim3: 257
+ - tim4: 258
+ - tim5: 259
+ - tim6: 260
+ - tim7: 261
+ - tim12: 262
+ - tim13: 263
+ - tim14: 264
+ - wwdg: 267
+ - spi2: 270
+ - spi3: 271
+ - uart2: 273
+ - uart3: 274
+ - uart4: 275
+ - uart5: 276
+ - i2c1: 277
+ - i2c2: 278
+ - i2c3: 279
+ - can1: 281
+ - can2: 282
+ - pwr: 284
+ - dac: 285
+ - uart7: 286
+ - uart8: 287
+ - tim1: 288
+ - tim8: 289
+ - usart1: 292
+ - usart6: 293
+ - adc: 296
+ - sdio: 299
+ - spi1: 300
+ - spi4: 301
+ - syscfg: 302
+ - tim9: 304
+ - tim10: 305
+ - tim11: 306
+ - spi5: 308
+ - spi6: 309
+ - sai1: 310
+ - ltdc: 314