Re: [PATCH v1 2/4] mhi_bus: controller: MHI support for QCOM modems

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

 





On 04/27/2018 04:32 AM, Arnd Bergmann wrote:
On Fri, Apr 27, 2018 at 4:23 AM, Sujeev Dias <sdias@xxxxxxxxxxxxxx> wrote:
QCOM PCIe based modems uses MHI as the communication protocol.
MHI control driver is the bus master for such modems. As the bus
master driver, it oversees power management operations
such as suspend, resume, powering on and off the device.

+- compatible
+  Usage: required
+  Value type: <string>
+  Definition: "qcom,mhi"
+
+- qcom,pci-dev-id
+  Usage: optional
+  Value type: <u32>
+  Definition: PCIe device id of external modem to bind. If not set, any
+       device is compatible with this node.
+
+- qcom,pci-domain
+  Usage: required
+  Value type: <u32>
+  Definition: PCIe root complex external modem connected to
+
+- qcom,pci-bus
+  Usage: required
+  Value type: <u32>
+  Definition: PCIe bus external modem connected to
+
+- qcom,pci-slot
+  Usage: required
+  Value type: <u32>
+  Definition: PCIe slot as assigned by pci framework to external modem
These don't seem to make any sense: You seem to have access to
a regular pci_device already, so why do you need to duplicate the
information about it in DT?

I will remove the platform device, original hardware design we had a complicated power on sequence that require platform device to come up first and follow a strict power on sequence to power on modem before pci device can enumerate. I stored the BDF in DT to correlate the platform device with pci device. platform device
is no longer needed so I can remove it.
+- qcom,smmu-cfg
+  Usage: required
+  Value type: <u32>
+  Definition: Required SMMU configuration bitmask for PCIe bus.
+       BIT mask:
+       BIT(0) : Attach address mapping to endpoint device
+       BIT(1) : Set attribute S1_BYPASS
+       BIT(2) : Set attribute FAST
+       BIT(3) : Set attribute ATOMIC
+       BIT(4) : Set attribute FORCE_COHERENT
+
+- qcom,addr-win
+  Usage: required if SMMU S1 translation is enabled
+  Value type: Array of <u64>
+  Definition: Pair of values describing iova start and stop address
Why do you need these? Can't that be handled by the PCI
layer?
I will move this to end point DT. PCIe end point driver does the iommu configuration
+- qcom,msm-bus,name
+  Usage: required
+  Value type: <string>
+  Definition: string representing the bus scale client name to register
This probably belongs into a separate binding for the bus
scale driver, right?
Yes, this is for qcom bus scale driver which I don't think is upstreamed yet. Will confirm.
+static struct pci_driver mhi_pcie_driver;
Please try to reorder the symbols to avoid forward declarations.

+static int mhi_platform_probe(struct platform_device *pdev)
+{
+       struct mhi_controller *mhi_cntrl;
+       struct mhi_dev *mhi_dev;
+       struct device_node *of_node = pdev->dev.of_node;
+       u64 addr_win[2];
+       int ret;
+
+       if (!of_node)
+               return -ENODEV;
+
+       mhi_cntrl = mhi_alloc_controller(sizeof(*mhi_dev));
+       if (!mhi_cntrl)
+               return -ENOMEM;
+
+       mhi_dev = mhi_controller_get_devdata(mhi_cntrl);
+
+       /* get pci bus topology for this node */
+       ret = of_property_read_u32(of_node, "qcom,pci-dev-id",
+                                  &mhi_cntrl->dev_id);
+       if (ret)
+               mhi_cntrl->dev_id = PCI_ANY_ID;
+
+       ret = of_property_read_u32(of_node, "qcom,pci-domain",
+                                  &mhi_cntrl->domain);
+       if (ret)
+               goto error_probe;
+
+       ret = of_property_read_u32(of_node, "qcom,pci-bus", &mhi_cntrl->bus);
+       if (ret)
+               goto error_probe;
+
+       ret = of_property_read_u32(of_node, "qcom,pci-slot", &mhi_cntrl->slot);
+       if (ret)
+               goto error_probe;
Please explain what you are trying to do here, why do you register
two device drivers? It looks like they both refer to the same
hardware, so why isn't it sufficient to have the pci_driver?
As I explained earlier, it's now. Original hardware design we had chicken egg situation where some driver has to come up and power on device before pcie enumeration can take place.


        Arnd
--
To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Thanks again
Sujeev

--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

--
To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[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