On 1/22/2019 9:12 AM, Uwe Kleine-König wrote:
> On Mon, Jan 21, 2019 at 10:50:04PM +0000, Leonard Crestez wrote:
>> On chips without a separate power domain for PCI (such as 6q/6qp) the
>> imx6_pcie_attach_pd function incorrectly returns an error.
>>
>> Fix by returning 0 if dev_pm_domain_attach_by_name doesn't find
>> anything.
>>
>> Fixes: 3f7cceeab895 ("PCI: imx: Add multi-pd support")
>>
>> diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c
>> @@ -308,10 +308,13 @@ static int imx6_pcie_attach_pd(struct device *dev)
>> return 0;
>>
>> imx6_pcie->pd_pcie = dev_pm_domain_attach_by_name(dev, "pcie");
>> if (IS_ERR(imx6_pcie->pd_pcie))
>> return PTR_ERR(imx6_pcie->pd_pcie);
>> + /* Do nothing when power domain missing */
>> + if (!imx6_pcie->pd_pcie)
>> + return 0;
>
> As I said in the mail that proposed this patch for testing: I think it
> would be better to change dev_pm_domain_attach_by_name to not return an
> error indication by returning NULL or an ERR_PTR value. (Or change
> device_link_add to accept NULL if NULL is a dummy value.)
>
> Just repeating it here to have it near the actual patch.
Link to that previous discussion:
http://lists.infradead.org/pipermail/linux-arm-kernel/2019-January/626859.html
I'm not sure what you mean. Should dev_pm_domain_attach_by_name return
ERR_PTR(-ENOENT) for name not found? It would still require special
handling for callers.
The device_link_add function already returns NULL if the consumer or
supplier is NULL but that's also the only way it signals failure. Maybe
that function should be adjusted to return ERR_PTR instead?
Changing core API for a driver bug seems inappropriate.
The current imx6_pcie_attach_pd code is complicated because it tries to
distinguish between "multi pd" and "no pd / single pd". I wish there was
a way for these PM_RUNTIME device_links to be setup automatically in
core and have multi-PD behave the same as a single PD.
There aren't many multi-PD users but commit 6494a9ad86de ("usb: xhci:
tegra: Add genpd support") seems to do pretty much the same thing.
--
Regards,
Leonard
