Re: [PATCH v3 2/2] spmi: pmic-arb: Add support for PMIC v7

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

 



On 01/02/2022 16:41, Vinod Koul wrote:
From: David Collins <collinsd@xxxxxxxxxxxxxx>

PMIC v7 has different offset values and seqeunces, so add support for
this new version of PMIC

Signed-off-by: David Collins <collinsd@xxxxxxxxxxxxxx>
Signed-off-by: Vinod Koul <vkoul@xxxxxxxxxx>
---
  drivers/spmi/spmi-pmic-arb.c | 233 ++++++++++++++++++++++++++++++++---
  1 file changed, 214 insertions(+), 19 deletions(-)


As I was asking Stephen about the fate of this patch series I could not stop my self from noticing that one of his comments ([1]) from v1 was ignored.
Let me quote it here:

> The driver is already pretty hard to read because it combines so many
> generations of spmi arbiter hardware from qcom into one file. It would
> probably be better to start over and simplify the new version of the
> driver, possibly sharing code between the two files if possible, but
> otherwise dropping lots of cruft along the way and simplifying review
> burden.

After taking a glance, I thought maybe we should really follow this approach. And it also allows us to start with the new bindings:

spmi@c400000 {
   compatible = "qcom,spmi-pmic-arb-v7";

   reg = <..... both arb registers as following....>;
reg-names = "core", "chnls", "observer", "cnfg0", "intr0", "cnfg1", "intr1";

   interrupts = <&pdc 1 HIGH>, <&pdc 3 HIGH>;
   interrupt-names = "arb0", "arb1"; /* are the names necessary at all? */

   #address-cells = <1>;
   #size-cells = <0>;

   spmi_bus: bus@0 {
      reg = <0>;
      #address-cells = <2>;
      #size-cells = <0>;
      #interrupt-cells = <4>;
      interrupt-controller;

      pmic@.... {
          // etc.
      };

   };

   spmi1_bus: bus@1 {
      reg = <1>;
      #address-cells = <2>;
      #size-cells = <0>;
      #interrupt-cells = <4>;
      interrupt-controller;
   };
};

Note, this drops the qcom,ee (which is always 0 for all devices I see in mainline)) and qcom,channel (which if I understood correctly is used only for pmic-arb-v1, ugh). It uses common reg = <N> property instead of cooked qcom,bus-id. And last, but not least, it save us from huuge comments in the source code telling why devm_platform_ioremap_resource can not be used.

[1] https://lore.kernel.org/linux-arm-msm/20211210020148.B2EA6C004DD@xxxxxxxxxxxxxxx/


--
With best wishes
Dmitry




[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux