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

Vinod Koul
2021-12-07T11:49:20Z

On 02-12-21, 15:51, David Collins wrote:
> On 12/2/21 3:06 PM, Stephen Boyd wrote:
> > Quoting Vinod Koul (2021-11-30 23:27:18)
> >> @@ -1169,8 +1270,12 @@ static int spmi_pmic_arb_probe(struct platform_device *pdev)
> >> pmic_arb = spmi_controller_get_drvdata(ctrl);
> >> pmic_arb->spmic = ctrl;
> >>
> >> + /*
> >> + * Don't use devm_ioremap_resource() as the resources are shared in
> >> + * PMIC v7 onwards, so causing failure when mapping
> >> + */
> >> res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "core");
> >> - core = devm_ioremap_resource(&ctrl->dev, res);
> >> + core = devm_ioremap(&ctrl->dev, res->start, resource_size(res));
> >
> > What does this mean? We have two nodes in DT that have the same reg
> > properties? How does that work?
>
> PMIC Arbiter v7 has two SPMI bus master interfaces. These are used to
> communicate with two sets of PMICs. The SPMI interfaces operate
> independently; however, they share some register address ranges (e.g.
> one common one is used for APID->PPID mapping). The most
> straightforward way to handle this is to treat them as two independent
> top-level DT devices.
>
> In this case the "cnfg" address is used in the DT node name as that is
> unique between the two instances.
>
> Here are the DT nodes used downstream on a target with PMIC Arbiter v7:
>
> spmi0_bus: qcom,spmi@c42d000 {
> compatible = "qcom,spmi-pmic-arb";
> reg = <0xc42d000 0x4000>,
> <0xc400000 0x3000>,
> <0xc500000 0x400000>,
> <0xc440000 0x80000>,
> <0xc4c0000 0x10000>;
> reg-names = "cnfg", "core", "chnls", "obsrvr", "intr";
> interrupts-extended = <&pdc 1 IRQ_TYPE_LEVEL_HIGH>;
> interrupt-names = "periph_irq";
> interrupt-controller;
> #interrupt-cells = <4>;
> #address-cells = <2>;
> #size-cells = <0>;
> cell-index = <0>;
> qcom,channel = <0>;
> qcom,ee = <0>;
> qcom,bus-id = <0>;
> };
>
> spmi1_bus: qcom,spmi@c432000 {
> compatible = "qcom,spmi-pmic-arb";
> reg = <0xc432000 0x4000>,
> <0xc400000 0x3000>,
> <0xc500000 0x400000>,
> <0xc440000 0x80000>,
> <0xc4d0000 0x10000>;
> reg-names = "cnfg", "core", "chnls", "obsrvr", "intr";
> interrupts-extended = <&pdc 3 IRQ_TYPE_LEVEL_HIGH>;
> interrupt-names = "periph_irq";
> interrupt-controller;
> #interrupt-cells = <4>;
> #address-cells = <2>;
> #size-cells = <0>;
> cell-index = <0>;
> qcom,channel = <0>;
> qcom,ee = <0>;
> qcom,bus-id = <1>;
> };
>
> Note the inclusion of a new DT property: "qcom,bus-id". This was
> defined in a DT binding patch that isn't present in Vinod's submission.
> Here is its definition:
>
> - qcom,bus-id : Specifies which SPMI bus instance to use. This property
> is only applicable for PMIC arbiter version 7 and
> beyond.
> Support values: 0 = primary bus, 1 = secondary bus
> Assumed to be 0 if unspecified.

I havent picked that part yet. This was not needed for base stuff to
work yet. Feel free to send that as additional change

--
~Vinod