Messages in this thread | | | Date | Tue, 30 Aug 2022 01:45:20 +0300 | Subject | Re: [PATCH v3 2/2] spmi: pmic-arb: Add support for PMIC v7 | From | Dmitry Baryshkov <> |
| |
On 01/02/2022 16:41, Vinod Koul wrote: > From: David Collins <collinsd@codeaurora.org> > > PMIC v7 has different offset values and seqeunces, so add support for > this new version of PMIC > > Signed-off-by: David Collins <collinsd@codeaurora.org> > Signed-off-by: Vinod Koul <vkoul@kernel.org> > --- > 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@smtp.kernel.org/
-- With best wishes Dmitry
| |