Re: [PATCH v2] dt-bindings: power: reset: qcom-pon: update "reg" property details

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On 6/29/2022 3:35 AM, Krzysztof Kozlowski wrote:
> On 28/06/2022 03:58, Anjelique Melendez wrote:
>> From: David Collins <quic_collinsd@xxxxxxxxxxx>
>>
>> Update the description of "reg" property to add the PON_PBS base
>> address along with PON_HLOS base address.  Also add "reg-names"
>> property description.
>>
>> Signed-off-by: David Collins <quic_collinsd@xxxxxxxxxxx>
>> Signed-off-by: Anjelique Melendez <quic_amelende@xxxxxxxxxxx>
>> ---
>>
>> New patch series to sperate this patch from applied patches.
>> Last comments from original patch series can be found
>> https://lore.kernel.org/linux-arm-msm/27515993-18f3-8891-4835-9b6a8d7f86b0@xxxxxxxxxxx/
>>
>> changes since v1:
>>   - Updated path which was missing Documention/devicetree prefix
>>   - Updated CC list
>>   
>>  ../bindings/power/reset/qcom,pon.yaml | 20 +++++++++++++++++++-
>>  1 file changed, 19 insertions(+), 1 deletion(-)
>>
>> diff --git a/Documentation/devicetree/bindings/power/reset/qcom,pon.yaml b/Documentation/devicetree/bindings/power/reset/qcom,pon.yaml
>> index 353f155d..1d8cf900 100644
>> --- a/Documentation/devicetree/bindings/power/reset/qcom,pon.yaml
>> +++ b/Documentation/devicetree/bindings/power/reset/qcom,pon.yaml
>> @@ -26,8 +26,26 @@ properties:
>>        - qcom,pm8998-pon
>>  
>>    reg:
>> -    maxItems: 1
>> +    description: |
>> +      Specifies the SPMI base address for the PON (power-on) peripheral.  For
>> +      PMICs that have the PON peripheral (GEN3) split into PON_HLOS and PON_PBS
>> +      (e.g. PMK8350), this can hold addresses of both PON_HLOS and PON_PBS
>> +      peripherals.  In that case, the PON_PBS address needs to be specified to
>> +      facilitate software debouncing on some PMICs.
> 
> You need separate compatibles for that and constraints (allOf:if:then)
> for each variant.
> 
So I have been looking into using the separate compatible strings however
have run into a few problems.

The compatible strings in this binding relate to the parent pon device,
while gen1/gen2/gen3 relate to the children devices.
For example, qcom,pm8941-resin/qcpm,pm8941-pwrkey are gen1/gen2 children
and defined under a parent pon device with the "qcom,pm8998-pon" compatible string
but qcom,pmk8350-resin/pmk8350-resin are gen3 children and would also be defined
under a parent pon device with the "qcom,pm8998-pon" compatible string.
Currently the children do not define their own "reg" property and will use the
register(s) address(es) defined for their parent[1].
Because of this we are unable to use the compatibles to separate the constraints
for gen1/gen2 vs gen3.

There are two possible suggestions we came up with that could solve this.
1. Add a new compatible string to this binding such as "qcom,pmk8350-pon". We would then
place all gen3 children devices under this compatible string thus being able to use
allOf:if:then for the constraints.This would also require a few changes within
driver/power/supply/qcom,pon.c. 

2. reg-names is not consumed by anything but rather used for clarification
for users as what the register(s) address relates to. We could get rid of
the property and simply have "reg" with minItem:1 and maxItem:2.


Would love to hear your thoughts or other suggestions you may have!

Thanks,
Anjelique

[1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/input/misc/pm8941-pwrkey.c?h=v5.19-rc4#n274
>> +    minItems: 1
>> +    maxItems: 2
>>  
>> +  reg-names:
>> +    description: |
>> +      For PON GEN1 and GEN2, it should be "pon". For PON GEN3 it should include
>> +      "pon_hlos" and optionally "pon_pbs".
>> +    minItems: 1
>> +    maxItems: 2
>> +    items:
>> +      anyOf:
>> +        - const: pon_hlos
>> +        - const: pon_pbs
>> +        - const: pon
> 
> First, you missed Rob's comment, that names should be without "pon" prefix.
> 
> Second, uUnfortunately this cannot be such flexible. Items have to be
> strictly ordered. You mentioned in description something about gen1/2/3,
> so probably what you want per-variant, specific list
> 
> Best regards,
> Krzysztof



[Index of Archives]     [Kernel Newbies]     [Security]     [Netfilter]     [Bugtraq]     [Linux FS]     [Yosemite Forum]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Video 4 Linux]     [Device Mapper]     [Linux Resources]

  Powered by Linux