Re: [PATCH v3 2/3] dt-bindings: iio: adc: Add support for QCOM PMIC5 Gen3 ADC

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

 



Hi Krzysztof,

On 2/17/2024 7:43 PM, Krzysztof Kozlowski wrote:
On 16/02/2024 11:39, Jishnu Prakash wrote:
Hi Krzysztof,

On 1/4/2024 1:48 PM, Krzysztof Kozlowski wrote:
On 31/12/2023 18:12, Jishnu Prakash wrote:
For the PMIC5-Gen3 type PMICs, ADC peripheral is present in HW for the
following PMICs: PMK8550, PM8550, PM8550B and PM8550VX PMICs.

It is similar to PMIC5-Gen2, with SW communication to ADCs on all PMICs
going through PBS(Programmable Boot Sequence) firmware through a single
register interface. This interface is implemented on an SDAM (Shared
Direct Access Memory) peripheral on the master PMIC PMK8550 rather
than a dedicated ADC peripheral.

Add documentation for PMIC5 Gen3 ADC and macro definitions for ADC
channels and virtual channels (combination of ADC channel number and
PMIC SID number) per PMIC, to be used by clients of this device.

Changes since v2:
- Moved ADC5 Gen3 documentation into a separate new file.
Changelog goes under ---.

Why did you do this? What is the rationale? Sorry, this patchset goes
nowhere.


I'll elaborate this more in the next patchset. There are two main
reasons for adding this documentation in a new file:

This was more than a month ago? You reply to my comment with 1.5 months
delay?

Sorry, I am not in the context and I am not going back to it. I have
many other emails where my questions are addressed faster than 1.5 months.

The patch is not even in my mailbox, long gone.
Why you are making it so difficult for reviewers?

You will get answers like I am not in context, sorry. Next time don't
respond after 1.5 months.


You're right - I'll do my best to get back to review comments in a reasonable time frame.



1.This device is not exactly like the existing QCOM VADC drivers as it
now combines VADC functionality (reading ADC channel on client request)
with ADC_TM functionality (thermal threshold monitoring).

Does no explain touching bindings. Your drivers don't matter for bindings.


2.Adding this device's bindings in the existing qcom,spmi-vadc.yaml file

No rationale was provided in commit msg.

is not possible as it would require updating some of the existing
top-level constraints. (for the older devices in that file, "reg" and
"interrupts" can have at most one item, while this device can have more
than one item under these properties.)


How is this a problem?

In qcom,spmi-vadc.yaml, we have the following top-level constraints for the "reg" and "interrupts" properties:

  reg:
    maxItems: 1

  interrupts:
    maxItems: 1

For the ADC5 Gen3 device being added now, these constraints cannot be followed always, as there may be more than one peripheral under one device instance, each with a corresponding interrupt. For example, the above properties could be like this for a ADC5 Gen3 device:

    reg = <0x9000>, <0x9100>;
    interrupts = <0x0 0x90 0x1 IRQ_TYPE_EDGE_RISING>,
                 <0x0 0x91 0x1 IRQ_TYPE_EDGE_RISING>;


I could not overwrite the top-level constraints for the new device "qcom,spmi-adc5-gen3" alone in qcom,spmi-vadc.yaml, so I tried to remove the constraints from the top level and add them back conditionally for all the device types separately, but you told me not to remove them (full message: https://lore.kernel.org/linux-iio/832053f4-bd5d-4e58-81bb-1a8188e7f364@xxxxxxxxxx/)

Since these constraints cannot be modified for a specific new device or removed, I think the only way to accommodate this new device is to add it in its own new file.

Is this a sufficient justification for adding this documentation in a new file or do you have any other suggestions?

Thanks,
Jishnu



Best regards,
Krzysztof





[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [Linux for Sparc]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux