Add support for vibrator module inside Qualcomm PMI632, PM7250B, PM7325B
PMICs.
Signed-off-by: Fenglin Wu <[email protected]>
---
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
reg:
maxItems: 1
--
2.25.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 <[email protected]>
> ---
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
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 <[email protected]>
>> ---
>
> 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
>
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 <[email protected]>
>>> ---
>>
>> 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/[email protected]/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
>>
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 <[email protected]>
>>>> ---
>>>
>>> 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/[email protected]/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
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/[email protected]/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
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 <[email protected]>
>>>>> ---
>>>>
>>>> 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/[email protected]/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
>
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 <[email protected]>
>>> ---
>>
>> 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
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 <[email protected]>
>>>> ---
>>>
>>> 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
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/[email protected]/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
>
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/[email protected]/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
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
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
>