LKML Archive on lore.kernel.org
 help / color / Atom feed
From: Arnd Bergmann <arnd@arndb.de>
To: Sujeev Dias <sdias@codeaurora.org>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	linux-arm-msm@vger.kernel.org,
	Tony Truong <truong@codeaurora.org>
Subject: Re: [PATCH v1 2/4] mhi_bus: controller: MHI support for QCOM modems
Date: Fri, 27 Apr 2018 13:32:28 +0200
Message-ID: <CAK8P3a3UhuFYDfNeQTba-E50yNyyjj9-1c03YhDVnYQ1DvNd1g@mail.gmail.com> (raw)
In-Reply-To: <1524795811-21399-3-git-send-email-sdias@codeaurora.org>

On Fri, Apr 27, 2018 at 4:23 AM, Sujeev Dias <sdias@codeaurora.org> 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?

> +- 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?

> +- 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?

> +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?

       Arnd

  reply index

Thread overview: 47+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-04-27  2:23 MHI initial design review Sujeev Dias
2018-04-27  2:23 ` [PATCH v1 1/4] mhi_bus: core: Add support for MHI host interface Sujeev Dias
2018-04-27  7:22   ` Greg Kroah-Hartman
2018-04-28 14:28     ` Sujeev Dias
2018-04-28 15:50       ` Greg Kroah-Hartman
2018-04-27  7:23   ` Greg Kroah-Hartman
2018-04-27 12:18   ` Arnd Bergmann
2018-04-28 16:08     ` Sujeev Dias
2018-04-28  0:28   ` kbuild test robot
2018-04-28  2:52   ` kbuild test robot
2018-05-03 19:21   ` Pavel Machek
2018-05-04  3:05     ` Sujeev Dias
2018-06-22 23:03   ` Randy Dunlap
2018-04-27  2:23 ` [PATCH v1 2/4] mhi_bus: controller: MHI support for QCOM modems Sujeev Dias
2018-04-27 11:32   ` Arnd Bergmann [this message]
2018-04-28 15:40     ` Sujeev Dias
2018-04-28  3:05   ` kbuild test robot
2018-04-28  3:12   ` kbuild test robot
2018-04-27  2:23 ` [PATCH v1 3/4] mhi_bus: dev: netdev: add network interface driver Sujeev Dias
2018-04-27 11:19   ` Arnd Bergmann
2018-04-28 15:25     ` Sujeev Dias
2018-04-27  2:23 ` [PATCH v1 4/4] mhi_bus: dev: uci: add user space " Sujeev Dias
2018-04-27 11:36   ` Arnd Bergmann
2018-04-28  1:03   ` kbuild test robot
2018-04-28  5:16   ` [PATCH] mhi_bus: dev: uci: fix semicolon.cocci warnings kbuild test robot
2018-04-28  5:16   ` [PATCH v1 4/4] mhi_bus: dev: uci: add user space interface driver kbuild test robot
2018-07-09 20:08 ` MHI code review Sujeev Dias
2018-07-09 20:08   ` [PATCH v2 1/7] mhi_bus: core: initial checkin for modem host interface bus driver Sujeev Dias
2018-07-09 20:50     ` Greg Kroah-Hartman
2018-07-09 20:52     ` Greg Kroah-Hartman
2018-07-10  6:36     ` Greg Kroah-Hartman
2018-07-11 19:30     ` Rob Herring
2018-08-09 18:39     ` Randy Dunlap
2018-07-09 20:08   ` [PATCH v2 2/7] mhi_bus: core: add power management support Sujeev Dias
2018-07-09 20:08   ` [PATCH v2 3/7] mhi_bus: core: add support for data transfer Sujeev Dias
2018-07-10  6:29     ` Greg Kroah-Hartman
2018-07-09 20:08   ` [PATCH v2 4/7] mhi_bus: core: add support for handling ioctl cmds Sujeev Dias
2018-07-09 20:08   ` [PATCH v2 5/7] mhi_bus: core: add support to get external modem time Sujeev Dias
2018-07-11 19:32     ` Rob Herring
2018-08-09 20:17     ` Randy Dunlap
2018-07-09 20:08   ` [PATCH v2 6/7] mhi_bus: controller: MHI support for QCOM modems Sujeev Dias
2018-07-11 19:36     ` Rob Herring
2018-07-09 20:08   ` [PATCH v2 7/7] mhi_bus: dev: uci: add user space interface driver Sujeev Dias
2019-04-30 15:10   ` MHI code review Daniele Palmas
2019-06-12 17:54     ` Sujeev Dias
2019-06-12 20:58       ` Daniele Palmas
2019-06-12 18:00     ` Sujeev Dias

Reply instructions:

You may reply publically to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CAK8P3a3UhuFYDfNeQTba-E50yNyyjj9-1c03YhDVnYQ1DvNd1g@mail.gmail.com \
    --to=arnd@arndb.de \
    --cc=gregkh@linuxfoundation.org \
    --cc=linux-arm-msm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=sdias@codeaurora.org \
    --cc=truong@codeaurora.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

LKML Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/lkml/0 lkml/git/0.git
	git clone --mirror https://lore.kernel.org/lkml/1 lkml/git/1.git
	git clone --mirror https://lore.kernel.org/lkml/2 lkml/git/2.git
	git clone --mirror https://lore.kernel.org/lkml/3 lkml/git/3.git
	git clone --mirror https://lore.kernel.org/lkml/4 lkml/git/4.git
	git clone --mirror https://lore.kernel.org/lkml/5 lkml/git/5.git
	git clone --mirror https://lore.kernel.org/lkml/6 lkml/git/6.git
	git clone --mirror https://lore.kernel.org/lkml/7 lkml/git/7.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 lkml lkml/ https://lore.kernel.org/lkml \
		linux-kernel@vger.kernel.org
	public-inbox-index lkml

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-kernel


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git