Hi,
Wanted to send a reminder for this patch conversation.
Thanks,
Anjelique
On 8/19/2022 1:26 PM, Anjelique Melendez wrote:
>
> Hi Krzysztof,
> First I would like to apologize for my lack of response to this patch series
> over these past few weeks. I have been out of office.
>
> On 7/26/2022 3:25 AM, Krzysztof Kozlowski wrote:
>> On 25/07/2022 21:13, 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 constraints.
>>>
>>> Signed-off-by: David Collins <quic_collinsd@xxxxxxxxxxx>
>>> Signed-off-by: Anjelique Melendez <quic_amelende@xxxxxxxxxxx>
>>> ---
>>> Documentation/devicetree/bindings/power/reset/qcom,pon.yaml | 50 +++++++++++++++++++++++++++---
>>> 1 file changed, 46 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/Documentation/devicetree/bindings/power/reset/qcom,pon.yaml b/Documentation/devicetree/bindings/power/reset/qcom,pon.yaml
>>> index 353f155d..d7b6b875 100644
>>> --- a/Documentation/devicetree/bindings/power/reset/qcom,pon.yaml
>>> +++ b/Documentation/devicetree/bindings/power/reset/qcom,pon.yaml
>>> @@ -15,18 +15,27 @@ description: |
>>>
>>> This DT node has pwrkey and resin as sub nodes.
>>>
>>> -allOf:
>>> - - $ref: reboot-mode.yaml#
>>> -
>>> properties:
>>> compatible:
>>> enum:
>>> - qcom,pm8916-pon
>>> - qcom,pms405-pon
>>> - qcom,pm8998-pon
>>> + - qcom,pmk8350-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 PMIC.
>>> + minItems: 1
>>> + maxItems: 2
>>> +
>>> + reg-names:
>>> + minItems: 1
>>> + maxItems: 2
>>>
>>> pwrkey:
>>> type: object
>>> @@ -42,6 +51,39 @@ required:
>>>
>>> unevaluatedProperties: false
>>>
>>> +allOf:
>>> + - $ref: reboot-mode.yaml#
>>> + - if:
>>> + properties:
>>> + compatible:
>>> + contains:
>>> + enum:
>>> + - qcom,pm8916-pon
>>> + - qcom,pms405-pon
>>> + - qcom,pm8998-pon
>>> + then:
>>> + properties:
>>> + reg:
>>> + maxItems: 1
>>> + reg-names:
>>> + items:
>>> + - const: pon
>>
>> All your previous patches were actually missing (in commit msg, in the
>> code) that piece of information which you add here. You now add
>> reg-names with "pon" for older devices. I assumed previous that it is
>> somehow needed, so I gave you the hints how it should be coded. But I
>> don't understand - why are you doing it
>>
>> This should be explained in commit msg. To me it is not needed at all...
>> unless you want to mark that first address space is entirely different
>> for other devices?
> Adding reg-names "pon" for older devices is simply to provide clarification
> about what the register relates to. Similar to reg-names "hlos" and "pbs"
> for gen3 children devices, reg-names is completely optional and is not
> consumed by any driver.
>
> Before adding the "qcom,pmk8350-pon" compatible string, the best way to
> differentiate between a "qcom,pm8998-pon" parent with a gen1/gen2 child vs a
> "qcom,pm8998-pon" parent with a gen3 child with only an hlos register was to
> include reg-names. i.e.
>
> pon_hlos@1300 {
> reg = <0x1300>;
> compatible = "qcom,pm8998-pon";
> reg-names = "pon";
> };
>
> vs
>
> pon_hlos@1300 {
> reg = <0x1300>;
> compatible = "qcom,pm8998-pon";
> reg-names = "hlos";
> };
>
> Adding the new "qcom,pmk8350-pon" compatible string would now be used to
> differentiate between gen1/gen2 vs gen3 children, so we could get rid of the
> addition of reg-names for older devices.
>
>
> Similarly we could get rid of reg-names and the "qcom,pmk8350-pon" compatible
> string as a whole like mentioned in
> https://lore.kernel.org/all/99a5d9ac-9c20-b441-44af-26772a0e989d@xxxxxxxxxx/,
> if reg-names and the new compatible string is causing too much confusion.
>
>
> Thanks,
> Anjelique
>>
>
>>> + - if:
>>> + properties:
>>> + compatible:
>>> + contains:
>>> + const: qcom,pmk8350-pon
>>> + then:
>>> + properties:
>>> + reg:
>>> + minItems: 1
>>> + maxItems: 2
>>> + reg-names:
>>> + minItems: 1
>>> + items:
>>> + - const: hlos
>>> + - const: pbs
>>> +
>>> examples:
>>> - |
>>> #include <dt-bindings/interrupt-controller/irq.h>
>>
>>
>> Best regards,
>> Krzysztof
