diff mbox series

[v2,1/2] dt-bindings: input: qcom,pm8xxx-vib: add more PMIC support

Message ID 20230718062639.2339589-2-quic_fenglinw@quicinc.com
State New
Headers
Series Add support for vibrator in multiple PMICs |

Commit Message

Fenglin Wu July 18, 2023, 6:26 a.m. UTC 
  Add support for vibrator module inside Qualcomm PMI632, PM7250B, PM7325B
PMICs.

Signed-off-by: Fenglin Wu <quic_fenglinw@quicinc.com>
---
 Documentation/devicetree/bindings/input/qcom,pm8xxx-vib.yaml | 3 +++
 1 file changed, 3 insertions(+)

Comments

Krzysztof Kozlowski July 18, 2023, 6:33 a.m. UTC | #1 
On 18/07/2023 08:26, Fenglin Wu wrote:
> Add support for vibrator module inside Qualcomm PMI632, PM7250B, PM7325B
> PMICs.
> 
> Signed-off-by: Fenglin Wu <quic_fenglinw@quicinc.com>
> ---

I don't see changelog. No changes then?

>  Documentation/devicetree/bindings/input/qcom,pm8xxx-vib.yaml | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/input/qcom,pm8xxx-vib.yaml b/Documentation/devicetree/bindings/input/qcom,pm8xxx-vib.yaml
> index c8832cd0d7da..481163105d24 100644
> --- a/Documentation/devicetree/bindings/input/qcom,pm8xxx-vib.yaml
> +++ b/Documentation/devicetree/bindings/input/qcom,pm8xxx-vib.yaml
> @@ -15,6 +15,9 @@ properties:
>        - qcom,pm8058-vib
>        - qcom,pm8916-vib
>        - qcom,pm8921-vib
> +      - qcom,pmi632-vib
> +      - qcom,pm7250b-vib
> +      - qcom,pm7325b-vib

Not much improved. With missing changelog, it seems you ignored the
feedback.


Best regards,
Krzysztof
Fenglin Wu July 18, 2023, 6:38 a.m. UTC | #2 
On 7/18/2023 2:33 PM, Krzysztof Kozlowski wrote:
> On 18/07/2023 08:26, Fenglin Wu wrote:
>> Add support for vibrator module inside Qualcomm PMI632, PM7250B, PM7325B
>> PMICs.
>>
>> Signed-off-by: Fenglin Wu <quic_fenglinw@quicinc.com>
>> ---
> 
> I don't see changelog. No changes then?
> 
Sorry, I updated the change log in the cover letter which didn't seems 
to be sent to a wider audience, I will resend it by adding more 
receivers in the to list

Fenglin
>>   Documentation/devicetree/bindings/input/qcom,pm8xxx-vib.yaml | 3 +++
>>   1 file changed, 3 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/input/qcom,pm8xxx-vib.yaml b/Documentation/devicetree/bindings/input/qcom,pm8xxx-vib.yaml
>> index c8832cd0d7da..481163105d24 100644
>> --- a/Documentation/devicetree/bindings/input/qcom,pm8xxx-vib.yaml
>> +++ b/Documentation/devicetree/bindings/input/qcom,pm8xxx-vib.yaml
>> @@ -15,6 +15,9 @@ properties:
>>         - qcom,pm8058-vib
>>         - qcom,pm8916-vib
>>         - qcom,pm8921-vib
>> +      - qcom,pmi632-vib
>> +      - qcom,pm7250b-vib
>> +      - qcom,pm7325b-vib
> 
> Not much improved. With missing changelog, it seems you ignored the
> feedback.
> 
> 
> Best regards,
> Krzysztof
>
Fenglin Wu July 18, 2023, 7:06 a.m. UTC | #3 
On 7/18/2023 2:38 PM, Fenglin Wu wrote:
> 
> 
> On 7/18/2023 2:33 PM, Krzysztof Kozlowski wrote:
>> On 18/07/2023 08:26, Fenglin Wu wrote:
>>> Add support for vibrator module inside Qualcomm PMI632, PM7250B, PM7325B
>>> PMICs.
>>>
>>> Signed-off-by: Fenglin Wu <quic_fenglinw@quicinc.com>
>>> ---
>>
>> I don't see changelog. No changes then?
>>
> Sorry, I updated the change log in the cover letter which didn't seems 
> to be sent to a wider audience, I will resend it by adding more 
> receivers in the to list
> 
> Fenglin

Just FYI,the change log was updated in the cover letter here: 
https://lore.kernel.org/linux-arm-msm/20230718062639.2339589-1-quic_fenglinw@quicinc.com/T/#m3819b50503ef19e0933a10bf797351a4af35537f

Also the commit text and the driver change were also updated accordingly 
to address your review comment by removing 'pm7550ba-vib' compatible string.

Since the changes are receiving review comments, I will not resend it. I 
will add a larger to-list when pushing the next patchset.

>>>   Documentation/devicetree/bindings/input/qcom,pm8xxx-vib.yaml | 3 +++
>>>   1 file changed, 3 insertions(+)
>>>
>>> diff --git 
>>> a/Documentation/devicetree/bindings/input/qcom,pm8xxx-vib.yaml 
>>> b/Documentation/devicetree/bindings/input/qcom,pm8xxx-vib.yaml
>>> index c8832cd0d7da..481163105d24 100644
>>> --- a/Documentation/devicetree/bindings/input/qcom,pm8xxx-vib.yaml
>>> +++ b/Documentation/devicetree/bindings/input/qcom,pm8xxx-vib.yaml
>>> @@ -15,6 +15,9 @@ properties:
>>>         - qcom,pm8058-vib
>>>         - qcom,pm8916-vib
>>>         - qcom,pm8921-vib
>>> +      - qcom,pmi632-vib
>>> +      - qcom,pm7250b-vib
>>> +      - qcom,pm7325b-vib
>>
>> Not much improved. With missing changelog, it seems you ignored the
>> feedback.
>>
>>
>> Best regards,
>> Krzysztof
>>
Krzysztof Kozlowski July 18, 2023, 7:20 a.m. UTC | #4 
On 18/07/2023 09:06, Fenglin Wu wrote:
> 
> 
> On 7/18/2023 2:38 PM, Fenglin Wu wrote:
>>
>>
>> On 7/18/2023 2:33 PM, Krzysztof Kozlowski wrote:
>>> On 18/07/2023 08:26, Fenglin Wu wrote:
>>>> Add support for vibrator module inside Qualcomm PMI632, PM7250B, PM7325B
>>>> PMICs.
>>>>
>>>> Signed-off-by: Fenglin Wu <quic_fenglinw@quicinc.com>
>>>> ---
>>>
>>> I don't see changelog. No changes then?
>>>
>> Sorry, I updated the change log in the cover letter which didn't seems 
>> to be sent to a wider audience, I will resend it by adding more 
>> receivers in the to list
>>
>> Fenglin
> 
> Just FYI,the change log was updated in the cover letter here: 
> https://lore.kernel.org/linux-arm-msm/20230718062639.2339589-1-quic_fenglinw@quicinc.com/T/#m3819b50503ef19e0933a10bf797351a4af35537f
> 
> Also the commit text and the driver change were also updated accordingly 
> to address your review comment by removing 'pm7550ba-vib' compatible string.

Removing compatible was never my feedback. Did you read:
https://elixir.bootlin.com/linux/v6.1-rc1/source/Documentation/devicetree/bindings/writing-bindings.rst#L42
?

Best regards,
Krzysztof
Fenglin Wu July 18, 2023, 7:59 a.m. UTC | #5 
On 7/18/2023 3:20 PM, Krzysztof Kozlowski wrote:
> On 18/07/2023 09:06, Fenglin Wu wrote:
>>
>>
>> On 7/18/2023 2:38 PM, Fenglin Wu wrote:
>>>
>>>
>>> On 7/18/2023 2:33 PM, Krzysztof Kozlowski wrote:
>>>> On 18/07/2023 08:26, Fenglin Wu wrote:
>>>>> Add support for vibrator module inside Qualcomm PMI632, PM7250B, PM7325B
>>>>> PMICs.
>>>>>
>>>>> Signed-off-by: Fenglin Wu <quic_fenglinw@quicinc.com>
>>>>> ---
>>>>
>>>> I don't see changelog. No changes then?
>>>>
>>> Sorry, I updated the change log in the cover letter which didn't seems
>>> to be sent to a wider audience, I will resend it by adding more
>>> receivers in the to list
>>>
>>> Fenglin
>>
>> Just FYI,the change log was updated in the cover letter here:
>> https://lore.kernel.org/linux-arm-msm/20230718062639.2339589-1-quic_fenglinw@quicinc.com/T/#m3819b50503ef19e0933a10bf797351a4af35537f
>>
>> Also the commit text and the driver change were also updated accordingly
>> to address your review comment by removing 'pm7550ba-vib' compatible string.
> 
> Removing compatible was never my feedback. Did you read:
> https://elixir.bootlin.com/linux/v6.1-rc1/source/Documentation/devicetree/bindings/writing-bindings.rst#L42
> ?
> 
Okay, so do you want me to add 'pm7550ba-vib' as a fallback compatible 
like this?

  properties:
    compatible:
-    enum:
-      - qcom,pm8058-vib
-      - qcom,pm8916-vib
-      - qcom,pm8921-vib
-      - qcom,pmi632-vib
-      - qcom,pm7250b-vib
-      - qcom,pm7325b-vib
+    oneOf:
+      - enum:
+          - qcom,pm8058-vib
+          - qcom,pm8916-vib
+          - qcom,pm8921-vib
+          - qcom,pmi632-vib
+          - qcom,pm7250b-vib
+          - qcom,pm7325b-vib
+      - items:
+          - enum:
+              - qcom,pm7550ba-vib
+          - const: qcom,pm7325b-vib



> Best regards,
> Krzysztof
>
Krzysztof Kozlowski July 18, 2023, 8:02 a.m. UTC | #6 
On 18/07/2023 09:59, Fenglin Wu wrote:

>>> Just FYI,the change log was updated in the cover letter here:
>>> https://lore.kernel.org/linux-arm-msm/20230718062639.2339589-1-quic_fenglinw@quicinc.com/T/#m3819b50503ef19e0933a10bf797351a4af35537f
>>>
>>> Also the commit text and the driver change were also updated accordingly
>>> to address your review comment by removing 'pm7550ba-vib' compatible string.
>>
>> Removing compatible was never my feedback. Did you read:
>> https://elixir.bootlin.com/linux/v6.1-rc1/source/Documentation/devicetree/bindings/writing-bindings.rst#L42
>> ?
>>
> Okay, so do you want me to add 'pm7550ba-vib' as a fallback compatible 
> like this?
> 
>   properties:
>     compatible:
> -    enum:
> -      - qcom,pm8058-vib
> -      - qcom,pm8916-vib
> -      - qcom,pm8921-vib
> -      - qcom,pmi632-vib
> -      - qcom,pm7250b-vib
> -      - qcom,pm7325b-vib
> +    oneOf:
> +      - enum:
> +          - qcom,pm8058-vib
> +          - qcom,pm8916-vib
> +          - qcom,pm8921-vib
> +          - qcom,pmi632-vib
> +          - qcom,pm7250b-vib
> +          - qcom,pm7325b-vib
> +      - items:
> +          - enum:
> +              - qcom,pm7550ba-vib
> +          - const: qcom,pm7325b-vib
> 

Yes

Best regards,
Krzysztof
Konrad Dybcio July 18, 2023, 10:51 a.m. UTC | #7 
On 18.07.2023 08:38, Fenglin Wu wrote:
> 
> 
> On 7/18/2023 2:33 PM, Krzysztof Kozlowski wrote:
>> On 18/07/2023 08:26, Fenglin Wu wrote:
>>> Add support for vibrator module inside Qualcomm PMI632, PM7250B, PM7325B
>>> PMICs.
>>>
>>> Signed-off-by: Fenglin Wu <quic_fenglinw@quicinc.com>
>>> ---
>>
>> I don't see changelog. No changes then?
>>
> Sorry, I updated the change log in the cover letter which didn't seems to be sent to a wider audience, I will resend it by adding more receivers in the to list
Please consider using the b4 tool which takes care of all that

https://b4.docs.kernel.org/en/latest/index.html

Konrad
Fenglin Wu July 18, 2023, 10:56 a.m. UTC | #8 
On 7/18/2023 6:51 PM, Konrad Dybcio wrote:
> On 18.07.2023 08:38, Fenglin Wu wrote:
>>
>>
>> On 7/18/2023 2:33 PM, Krzysztof Kozlowski wrote:
>>> On 18/07/2023 08:26, Fenglin Wu wrote:
>>>> Add support for vibrator module inside Qualcomm PMI632, PM7250B, PM7325B
>>>> PMICs.
>>>>
>>>> Signed-off-by: Fenglin Wu <quic_fenglinw@quicinc.com>
>>>> ---
>>>
>>> I don't see changelog. No changes then?
>>>
>> Sorry, I updated the change log in the cover letter which didn't seems to be sent to a wider audience, I will resend it by adding more receivers in the to list
> Please consider using the b4 tool which takes care of all that
> 
> https://b4.docs.kernel.org/en/latest/index.html
> 
Thanks Konrad, I will check and update at my side.

> Konrad
Krzysztof Kozlowski July 27, 2023, 7:10 a.m. UTC | #9 
On 18/07/2023 10:02, Krzysztof Kozlowski wrote:
> On 18/07/2023 09:59, Fenglin Wu wrote:
> 
>>>> Just FYI,the change log was updated in the cover letter here:
>>>> https://lore.kernel.org/linux-arm-msm/20230718062639.2339589-1-quic_fenglinw@quicinc.com/T/#m3819b50503ef19e0933a10bf797351a4af35537f
>>>>
>>>> Also the commit text and the driver change were also updated accordingly
>>>> to address your review comment by removing 'pm7550ba-vib' compatible string.
>>>
>>> Removing compatible was never my feedback. Did you read:
>>> https://elixir.bootlin.com/linux/v6.1-rc1/source/Documentation/devicetree/bindings/writing-bindings.rst#L42
>>> ?
>>>
>> Okay, so do you want me to add 'pm7550ba-vib' as a fallback compatible 
>> like this?
>>
>>   properties:
>>     compatible:
>> -    enum:
>> -      - qcom,pm8058-vib
>> -      - qcom,pm8916-vib
>> -      - qcom,pm8921-vib
>> -      - qcom,pmi632-vib
>> -      - qcom,pm7250b-vib
>> -      - qcom,pm7325b-vib
>> +    oneOf:
>> +      - enum:
>> +          - qcom,pm8058-vib
>> +          - qcom,pm8916-vib
>> +          - qcom,pm8921-vib
>> +          - qcom,pmi632-vib
>> +          - qcom,pm7250b-vib
>> +          - qcom,pm7325b-vib
>> +      - items:
>> +          - enum:
>> +              - qcom,pm7550ba-vib
>> +          - const: qcom,pm7325b-vib
>>
> 
> Yes

I wonder why this approved change turned out to something incorrect in
your v3 patch...

Best regards,
Krzysztof
Fenglin Wu July 27, 2023, 7:54 a.m. UTC | #10 
On 7/27/2023 3:10 PM, Krzysztof Kozlowski wrote:
> On 18/07/2023 10:02, Krzysztof Kozlowski wrote:
>> On 18/07/2023 09:59, Fenglin Wu wrote:
>>
>>>>> Just FYI,the change log was updated in the cover letter here:
>>>>> https://lore.kernel.org/linux-arm-msm/20230718062639.2339589-1-quic_fenglinw@quicinc.com/T/#m3819b50503ef19e0933a10bf797351a4af35537f
>>>>>
>>>>> Also the commit text and the driver change were also updated accordingly
>>>>> to address your review comment by removing 'pm7550ba-vib' compatible string.
>>>>
>>>> Removing compatible was never my feedback. Did you read:
>>>> https://elixir.bootlin.com/linux/v6.1-rc1/source/Documentation/devicetree/bindings/writing-bindings.rst#L42
>>>> ?
>>>>
>>> Okay, so do you want me to add 'pm7550ba-vib' as a fallback compatible
>>> like this?
>>>
>>>    properties:
>>>      compatible:
>>> -    enum:
>>> -      - qcom,pm8058-vib
>>> -      - qcom,pm8916-vib
>>> -      - qcom,pm8921-vib
>>> -      - qcom,pmi632-vib
>>> -      - qcom,pm7250b-vib
>>> -      - qcom,pm7325b-vib
>>> +    oneOf:
>>> +      - enum:
>>> +          - qcom,pm8058-vib
>>> +          - qcom,pm8916-vib
>>> +          - qcom,pm8921-vib
>>> +          - qcom,pmi632-vib
>>> +          - qcom,pm7250b-vib
>>> +          - qcom,pm7325b-vib
>>> +      - items:
>>> +          - enum:
>>> +              - qcom,pm7550ba-vib
>>> +          - const: qcom,pm7325b-vib
>>>
>>
>> Yes
> 
> I wonder why this approved change turned out to something incorrect in
> your v3 patch...
> 
Since I got review comments in the driver change and I was told to 
refactor the driver before adding new HW support. I added the HW type 
logic in the driver and I was thinking it might be good to add some 
generic compatible strings to match with the HW type introduced in the 
driver change.

Anyway, I will update it to what you suggested in next patch.

> Best regards,
> Krzysztof
>
Krzysztof Kozlowski July 27, 2023, 8:29 a.m. UTC | #11 
On 27/07/2023 09:54, Fenglin Wu wrote:
>>>> +          - enum:
>>>> +              - qcom,pm7550ba-vib
>>>> +          - const: qcom,pm7325b-vib
>>>>
>>>
>>> Yes
>>
>> I wonder why this approved change turned out to something incorrect in
>> your v3 patch...
>>
> Since I got review comments in the driver change and I was told to 
> refactor the driver before adding new HW support. I added the HW type 
> logic in the driver and I was thinking it might be good to add some 
> generic compatible strings to match with the HW type introduced in the 
> driver change.
> 
> Anyway, I will update it to what you suggested in next patch.

Drivers are not really related to bindings, so whatever HW type you add
in driver, is not a reason to change bindings. Reason to change bindings
could be for example: because hardware is like that.

Best regards,
Krzysztof
Fenglin Wu July 27, 2023, 10:51 a.m. UTC | #12 
On 7/27/2023 4:29 PM, Krzysztof Kozlowski wrote:
> On 27/07/2023 09:54, Fenglin Wu wrote:
>>>>> +          - enum:
>>>>> +              - qcom,pm7550ba-vib
>>>>> +          - const: qcom,pm7325b-vib
>>>>>
>>>>
>>>> Yes
>>>
>>> I wonder why this approved change turned out to something incorrect in
>>> your v3 patch...
>>>
>> Since I got review comments in the driver change and I was told to
>> refactor the driver before adding new HW support. I added the HW type
>> logic in the driver and I was thinking it might be good to add some
>> generic compatible strings to match with the HW type introduced in the
>> driver change.
>>
>> Anyway, I will update it to what you suggested in next patch.
> 
> Drivers are not really related to bindings, so whatever HW type you add
> in driver, is not a reason to change bindings. Reason to change bindings
> could be for example: because hardware is like that.
> 
Understood.

Sorry, I forgot to mention, in v3, I added the 'reg' value to the 
register offset and no longer hard code the 16-bit register address, 
that makes the vibrators inside PMI632/PM7250B/PM7325B/PM7550BA all 
compatible, and that was another motivation of adding a generic 
compatible string and make the others as the fallback.

This will be still the case in v4, I might keep it similar in v3 but 
just drop "qcom,spmi-vib-gen1"

> Best regards,
> Krzysztof
>
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/input/qcom,pm8xxx-vib.yaml b/Documentation/devicetree/bindings/input/qcom,pm8xxx-vib.yaml
index c8832cd0d7da..481163105d24 100644
--- a/Documentation/devicetree/bindings/input/qcom,pm8xxx-vib.yaml
+++ b/Documentation/devicetree/bindings/input/qcom,pm8xxx-vib.yaml
@@ -15,6 +15,9 @@  properties:
       - qcom,pm8058-vib
       - qcom,pm8916-vib
       - qcom,pm8921-vib
+      - qcom,pmi632-vib
+      - qcom,pm7250b-vib
+      - qcom,pm7325b-vib
 
   reg:
     maxItems: 1