[v1,1/2] dt-bindings: Sync the dts to this document
diff mbox

Message ID 1445395380-5365-2-git-send-email-wxt@rock-chips.com
State New
Headers show

Commit Message

Caesar Wang Oct. 21, 2015, 2:42 a.m. UTC
Add the OTP gpio state, we need switch the pin to gpio state
before the TSADC controller is reset.

Signed-off-by: Caesar Wang <wxt@rock-chips.com>
---

Changes in v1:
  - As the Doug comments, add the 'init' property to sync document.

 Documentation/devicetree/bindings/thermal/rockchip-thermal.txt | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

Comments

Doug Anderson Oct. 21, 2015, 4:23 a.m. UTC | #1
Hi,

On Tue, Oct 20, 2015 at 7:42 PM, Caesar Wang <wxt@rock-chips.com> wrote:
> Add the OTP gpio state, we need switch the pin to gpio state
> before the TSADC controller is reset.
>
> Signed-off-by: Caesar Wang <wxt@rock-chips.com>
> ---
>
> Changes in v1:
>   - As the Doug comments, add the 'init' property to sync document.
>
>  Documentation/devicetree/bindings/thermal/rockchip-thermal.txt | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)

Seems reasonable to me.  I do wonder if we need to make it more
obvious that things might glitch if the "init" pinctrl isn't there?
...probably not too critical unless others think it needs to be more
obvious...

Reviewed-by: Douglas Anderson <dianders@chromium.org>
Rob Herring Oct. 21, 2015, 3:18 p.m. UTC | #2
On Tue, Oct 20, 2015 at 9:42 PM, Caesar Wang <wxt@rock-chips.com> wrote:
> Add the OTP gpio state, we need switch the pin to gpio state
> before the TSADC controller is reset.
>
> Signed-off-by: Caesar Wang <wxt@rock-chips.com>
> ---
>
> Changes in v1:
>   - As the Doug comments, add the 'init' property to sync document.
>
>  Documentation/devicetree/bindings/thermal/rockchip-thermal.txt | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/thermal/rockchip-thermal.txt b/Documentation/devicetree/bindings/thermal/rockchip-thermal.txt
> index ef802de..28e84f7 100644
> --- a/Documentation/devicetree/bindings/thermal/rockchip-thermal.txt
> +++ b/Documentation/devicetree/bindings/thermal/rockchip-thermal.txt
> @@ -27,8 +27,9 @@ tsadc: tsadc@ff280000 {
>         clock-names = "tsadc", "apb_pclk";
>         resets = <&cru SRST_TSADC>;
>         reset-names = "tsadc-apb";
> -       pinctrl-names = "default";
> -       pinctrl-0 = <&otp_out>;
> +       pinctrl-names = "init", "default";
> +       pinctrl-0 = <&otp_gpio>;
> +       pinctrl-1 = <&otp_out>;

Are these optional or required? They only appear in the example.

Rob
Caesar Wang Oct. 21, 2015, 3:45 p.m. UTC | #3
Hi Rob,

? 2015?10?21? 23:18, Rob Herring ??:
> On Tue, Oct 20, 2015 at 9:42 PM, Caesar Wang <wxt@rock-chips.com> wrote:
>> Add the OTP gpio state, we need switch the pin to gpio state
>> before the TSADC controller is reset.
>>
>> Signed-off-by: Caesar Wang <wxt@rock-chips.com>
>> ---
>>
>> Changes in v1:
>>    - As the Doug comments, add the 'init' property to sync document.
>>
>>   Documentation/devicetree/bindings/thermal/rockchip-thermal.txt | 5 +++--
>>   1 file changed, 3 insertions(+), 2 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/thermal/rockchip-thermal.txt b/Documentation/devicetree/bindings/thermal/rockchip-thermal.txt
>> index ef802de..28e84f7 100644
>> --- a/Documentation/devicetree/bindings/thermal/rockchip-thermal.txt
>> +++ b/Documentation/devicetree/bindings/thermal/rockchip-thermal.txt
>> @@ -27,8 +27,9 @@ tsadc: tsadc@ff280000 {
>>          clock-names = "tsadc", "apb_pclk";
>>          resets = <&cru SRST_TSADC>;
>>          reset-names = "tsadc-apb";
>> -       pinctrl-names = "default";
>> -       pinctrl-0 = <&otp_out>;
>> +       pinctrl-names = "init", "default";
>> +       pinctrl-0 = <&otp_gpio>;
>> +       pinctrl-1 = <&otp_out>;
> Are these optional or required? They only appear in the example.

Yep,
These are required for TSADC.

I‘m assumed that's right,
I think we don't need to introduce the pinctrl in this document.

> Rob
>
> _______________________________________________
> Linux-rockchip mailing list
> Linux-rockchip@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-rockchip
Rob Herring Oct. 21, 2015, 4:48 p.m. UTC | #4
On Wed, Oct 21, 2015 at 10:45 AM, Caesar Wang <caesar.upstream@gmail.com> wrote:
> Hi Rob,
>
> ? 2015?10?21? 23:18, Rob Herring ??:
>>
>> On Tue, Oct 20, 2015 at 9:42 PM, Caesar Wang <wxt@rock-chips.com> wrote:
>>>
>>> Add the OTP gpio state, we need switch the pin to gpio state
>>> before the TSADC controller is reset.
>>>
>>> Signed-off-by: Caesar Wang <wxt@rock-chips.com>
>>> ---
>>>
>>> Changes in v1:
>>>    - As the Doug comments, add the 'init' property to sync document.
>>>
>>>   Documentation/devicetree/bindings/thermal/rockchip-thermal.txt | 5
>>> +++--
>>>   1 file changed, 3 insertions(+), 2 deletions(-)
>>>
>>> diff --git
>>> a/Documentation/devicetree/bindings/thermal/rockchip-thermal.txt
>>> b/Documentation/devicetree/bindings/thermal/rockchip-thermal.txt
>>> index ef802de..28e84f7 100644
>>> --- a/Documentation/devicetree/bindings/thermal/rockchip-thermal.txt
>>> +++ b/Documentation/devicetree/bindings/thermal/rockchip-thermal.txt
>>> @@ -27,8 +27,9 @@ tsadc: tsadc@ff280000 {
>>>          clock-names = "tsadc", "apb_pclk";
>>>          resets = <&cru SRST_TSADC>;
>>>          reset-names = "tsadc-apb";
>>> -       pinctrl-names = "default";
>>> -       pinctrl-0 = <&otp_out>;
>>> +       pinctrl-names = "init", "default";
>>> +       pinctrl-0 = <&otp_gpio>;
>>> +       pinctrl-1 = <&otp_out>;
>>
>> Are these optional or required? They only appear in the example.
>
>
> Yep,
> These are required for TSADC.

Then list them that way.

>
> I‘m assumed that's right,
> I think we don't need to introduce the pinctrl in this document.

You need to state that you use them and what valid names are. That is
a contract with the driver.

The example is extra information and should not be required to write a
dts entry.

Rob
Rob Herring Oct. 22, 2015, 1:18 a.m. UTC | #5
On Wed, Oct 21, 2015 at 11:48 AM, Rob Herring <robh+dt@kernel.org> wrote:
> On Wed, Oct 21, 2015 at 10:45 AM, Caesar Wang <caesar.upstream@gmail.com> wrote:
>> Hi Rob,
>>
>> ? 2015?10?21? 23:18, Rob Herring ??:
>>>
>>> On Tue, Oct 20, 2015 at 9:42 PM, Caesar Wang <wxt@rock-chips.com> wrote:
>>>>
>>>> Add the OTP gpio state, we need switch the pin to gpio state
>>>> before the TSADC controller is reset.

Also, please fix the subject to be specific what binding you are changing.

dt-bindings: rockchip-thermal: <what the change is>

Rob
Caesar Wang Oct. 22, 2015, 1:27 a.m. UTC | #6
? 2015?10?22? 09:18, Rob Herring ??:
> On Wed, Oct 21, 2015 at 11:48 AM, Rob Herring <robh+dt@kernel.org> wrote:
>> On Wed, Oct 21, 2015 at 10:45 AM, Caesar Wang <caesar.upstream@gmail.com> wrote:
>>> Hi Rob,
>>>
>>> ? 2015?10?21? 23:18, Rob Herring ??:
>>>> On Tue, Oct 20, 2015 at 9:42 PM, Caesar Wang <wxt@rock-chips.com> wrote:
>>>>> Add the OTP gpio state, we need switch the pin to gpio state
>>>>> before the TSADC controller is reset.
> Also, please fix the subject to be specific what binding you are changing.
>
> dt-bindings: rockchip-thermal: <what the change is>

Sorry, Mr. Rob
I just sent the patch.:-(


> Rob
>
>
>
>
> -- 
> Thanks,
> Caesar

Patch
diff mbox

diff --git a/Documentation/devicetree/bindings/thermal/rockchip-thermal.txt b/Documentation/devicetree/bindings/thermal/rockchip-thermal.txt
index ef802de..28e84f7 100644
--- a/Documentation/devicetree/bindings/thermal/rockchip-thermal.txt
+++ b/Documentation/devicetree/bindings/thermal/rockchip-thermal.txt
@@ -27,8 +27,9 @@  tsadc: tsadc@ff280000 {
 	clock-names = "tsadc", "apb_pclk";
 	resets = <&cru SRST_TSADC>;
 	reset-names = "tsadc-apb";
-	pinctrl-names = "default";
-	pinctrl-0 = <&otp_out>;
+	pinctrl-names = "init", "default";
+	pinctrl-0 = <&otp_gpio>;
+	pinctrl-1 = <&otp_out>;
 	#thermal-sensor-cells = <1>;
 	rockchip,hw-tshut-temp = <95000>;
 	rockchip,hw-tshut-mode = <0>;