On 13/02/2024 15:41, Abel Vesa wrote:
The v7 HW supports currently 2 buses. So register each bus as a separate
spmi controller and adapt all ops to use the bus instead of the
arbitrator. Legacy mode is still supported as long as there is no child
node that represents a bus, instead all nodes are expected to be actual
slave devices.
Signed-off-by: Abel Vesa <abel.vesa@xxxxxxxxxx>
---
Changes in v2:
- Reworked it so that it registers a spmi controller for each bus
rather than relying on the generic framework to pass on the bus
(master) id.
Thanks, I think this is better.
- Link to v1: https://lore.kernel.org/r/20240207-spmi-multi-master-support-v1-0-ce57f301c7fd@xxxxxxxxxx
---
drivers/spmi/spmi-pmic-arb.c | 950 ++++++++++++++++++++++++++-----------------
1 file changed, 585 insertions(+), 365 deletions(-)
diff --git a/drivers/spmi/spmi-pmic-arb.c b/drivers/spmi/spmi-pmic-arb.c
index 9ed1180fe31f..eced35b712b4 100644
--- a/drivers/spmi/spmi-pmic-arb.c
+++ b/drivers/spmi/spmi-pmic-arb.c
<snip>
+static int spmi_pmic_arb_bus_init(struct platform_device *pdev,
+ struct device_node *node,
+ struct spmi_pmic_arb *pmic_arb)
+{
+ int bus_index = pmic_arb->buses_available;
+ struct spmi_pmic_arb_bus *bus = &pmic_arb->buses[bus_index];
+ struct device *dev = &pdev->dev;
+ struct spmi_controller *ctrl;
+ void __iomem *intr;
+ void __iomem *cnfg;
+ int index, ret;
+ u32 irq;
+
+ ctrl = devm_spmi_controller_alloc(dev, sizeof(*ctrl));
+ if (IS_ERR(ctrl))
+ return PTR_ERR(ctrl);
+
+ ctrl->cmd = pmic_arb_cmd;
+ ctrl->read_cmd = pmic_arb_read_cmd;
+ ctrl->write_cmd = pmic_arb_write_cmd;
+
+ bus = spmi_controller_get_drvdata(ctrl);
+ bus->spmic = ctrl;
+
+ bus->ppid_to_apid = devm_kcalloc(dev, PMIC_ARB_MAX_PPID,
+ sizeof(*bus->ppid_to_apid),
+ GFP_KERNEL);
+ if (!bus->ppid_to_apid)
+ return -ENOMEM;
+
+ bus->apid_data = devm_kcalloc(dev, pmic_arb->max_periphs,
+ sizeof(*bus->apid_data),
+ GFP_KERNEL);
+ if (!bus->apid_data)
+ return -ENOMEM;
+
+ /* Optional property for v7: */
+ of_property_read_u32(node, "qcom,bus-id", &bus_index);
Not sure what bindings you plan to use, but this should be the reg property.
+ if (bus_index != pmic_arb->buses_available) {
+ dev_err(dev, "wrong bus-id value");
+ return -EINVAL;
+ }
+
+ index = of_property_match_string(node, "reg-names", "cnfg");
+ if (index < 0) {
+ dev_err(dev, "cnfg reg region missing");
+ return -EINVAL;
+ }
+
+ cnfg = devm_of_iomap(dev, node, index, NULL);
+ if (IS_ERR(cnfg))
+ return PTR_ERR(cnfg);
+
+ index = of_property_match_string(node, "reg-names", "intr");
+ if (index < 0) {
+ dev_err(dev, "intr reg region missing");
+ return -EINVAL;
+ }
+
+ intr = devm_of_iomap(dev, node, index, NULL);
+ if (IS_ERR(intr))
+ return PTR_ERR(intr);
+
+ irq = of_irq_get_byname(node, "periph_irq");
+ if (irq < 0)
+ return irq;
+
+ bus->pmic_arb = pmic_arb;
+ bus->intr = intr;
+ bus->cnfg = cnfg;
+ bus->irq = irq;
+ bus->id = bus_index;
+
+ ret = pmic_arb->ver_ops->init_apid(bus, index);
+ if (ret)
+ return ret;
+
+ dev_dbg(&pdev->dev, "adding irq domain for bus %d\n", bus_index);
+
+ bus->domain = irq_domain_add_tree(dev->of_node,
+ &pmic_arb_irq_domain_ops, bus);
+ if (!bus->domain) {
+ dev_err(&pdev->dev, "unable to create irq_domain\n");
+ return -ENOMEM;
+ }
+
+ irq_set_chained_handler_and_data(bus->irq,
+ pmic_arb_chained_irq, bus);
+
+ bus->spmic->dev.of_node = node;
+ dev_set_name(&bus->spmic->dev, "spmi-%d", bus_index);
+
+ ret = devm_spmi_controller_add(dev, bus->spmic);
+ if (ret)
+ return ret;
+
+ pmic_arb->buses_available++;
+
+ return 0;
+}
+
+static int spmi_pmic_arb_register_buses(struct spmi_pmic_arb *pmic_arb,
+ struct platform_device *pdev)
+{
+ struct device *dev = &pdev->dev;
+ struct device_node *node = dev->of_node;
+ struct device_node *child;
+ int ret;
+
+ for_each_available_child_of_node(node, child)
+ if (of_node_name_eq(child, "bus")) {
It seems you use "bus" subnodes, it seems you should also submit a new
bindings scheme for v7 controllers with subnodes
+ ret = spmi_pmic_arb_bus_init(pdev, child, pmic_arb);
+ if (ret)
+ return ret;
+ }
+
+ if (!pmic_arb->buses_available)
+ ret = spmi_pmic_arb_bus_init(pdev, node, pmic_arb);
+
+ return ret;
+}
+
+static void spmi_pmic_arb_deregister_buses(struct spmi_pmic_arb *pmic_arb)
+{
+ int i;
+
+ for (i = 0; i < pmic_arb->buses_available; i++) {
+ struct spmi_pmic_arb_bus *bus = &pmic_arb->buses[i];
+
+ irq_set_chained_handler_and_data(bus->irq,
+ NULL, NULL);
+ irq_domain_remove(bus->domain);
+ }
+}
+
<snip>
Overall the patch is __huge__, could you split it ? Like move the bus handling
then add the multi-bus support in separate patches ?
But first please add new bindings first so we know what you expect from DT.
Thanks,
Neil
---
base-commit: 445a555e0623387fa9b94e68e61681717e70200a
change-id: 20240207-spmi-multi-master-support-832a704b779b
Best regards,
