| Message ID | 1452667666-17533-4-git-send-email-kishon@ti.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
* Kishon Vijay Abraham I <kishon@ti.com> [160112 22:48]: > Use platform populated reset assert and deassert > callbacks to perform reset of PCIe. > > Use these callbacks until a reset interface using drivers/reset > is available for the purpose. This one has a dependency to the second patch for the platform data. Bjorn, how do you prefer to merge this once there are no more comments? How about I set up an immutable branch against v4.5-rc1 with just these three patches that we can both then merge in? My preference is to add this to linux next after the merge window for v4.6. Bjorn, if you want it merged as fixes, I'm fine with that too naturally. Regards, Tony -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 01/13/2016 12:47 AM, Kishon Vijay Abraham I wrote: > Use platform populated reset assert and deassert > callbacks to perform reset of PCIe. > > Use these callbacks until a reset interface using drivers/reset > is available for the purpose. > > Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com> > Signed-off-by: Sekhar Nori <nsekhar@ti.com> > --- > drivers/pci/host/pci-dra7xx.c | 32 ++++++++++++++++++++++++++++++++ > 1 file changed, 32 insertions(+) > > diff --git a/drivers/pci/host/pci-dra7xx.c b/drivers/pci/host/pci-dra7xx.c > index 8c36880..049083d 100644 > --- a/drivers/pci/host/pci-dra7xx.c > +++ b/drivers/pci/host/pci-dra7xx.c > @@ -25,6 +25,8 @@ > #include <linux/resource.h> > #include <linux/types.h> > > +#include <linux/platform_data/pci-dra7xx.h> > + > #include "pcie-designware.h" > > /* PCIe controller wrapper DRA7XX configuration registers */ > @@ -329,6 +331,32 @@ static int __init dra7xx_add_pcie_port(struct dra7xx_pcie *dra7xx, > return 0; > } > > +static int dra7xx_pcie_reset(struct platform_device *pdev) > +{ > + int ret; > + struct device *dev = &pdev->dev; > + struct pci_dra7xx_platform_data *pdata = pdev->dev.platform_data; > + > + if (!(pdata && pdata->deassert_reset && pdata->assert_reset)) { > + dev_err(dev, "platform data for reset not found!\n"); > + return -EINVAL; > + } > + > + ret = pdata->assert_reset(pdev, pdata->reset_name); > + if (ret) { > + dev_err(dev, "assert_reset failed: %d\n", ret); > + return ret; > + } > + > + ret = pdata->deassert_reset(pdev, pdata->reset_name); > + if (ret) { > + dev_err(dev, "deassert_reset failed: %d\n", ret); > + return ret; > + } The only comment I have on this is the symmetry (assert_reset invocation in driver remove). If you install and remove the module once, then the reset stays deasserted. On Power-On-Reset, the resets by default will be in asserted state. regards SUman > + > + return 0; > +} > + > static int __init dra7xx_pcie_probe(struct platform_device *pdev) > { > u32 reg; > @@ -347,6 +375,10 @@ static int __init dra7xx_pcie_probe(struct platform_device *pdev) > enum of_gpio_flags flags; > unsigned long gpio_flags; > > + ret = dra7xx_pcie_reset(pdev); > + if (ret) > + return ret; > + > dra7xx = devm_kzalloc(dev, sizeof(*dra7xx), GFP_KERNEL); > if (!dra7xx) > return -ENOMEM; > -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Suman, On Wednesday 13 January 2016 11:21 PM, Suman Anna wrote: > On 01/13/2016 12:47 AM, Kishon Vijay Abraham I wrote: >> Use platform populated reset assert and deassert >> callbacks to perform reset of PCIe. >> >> Use these callbacks until a reset interface using drivers/reset >> is available for the purpose. >> >> Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com> >> Signed-off-by: Sekhar Nori <nsekhar@ti.com> >> --- >> drivers/pci/host/pci-dra7xx.c | 32 ++++++++++++++++++++++++++++++++ >> 1 file changed, 32 insertions(+) >> >> diff --git a/drivers/pci/host/pci-dra7xx.c b/drivers/pci/host/pci-dra7xx.c >> index 8c36880..049083d 100644 >> --- a/drivers/pci/host/pci-dra7xx.c >> +++ b/drivers/pci/host/pci-dra7xx.c >> @@ -25,6 +25,8 @@ >> #include <linux/resource.h> >> #include <linux/types.h> >> >> +#include <linux/platform_data/pci-dra7xx.h> >> + >> #include "pcie-designware.h" >> >> /* PCIe controller wrapper DRA7XX configuration registers */ >> @@ -329,6 +331,32 @@ static int __init dra7xx_add_pcie_port(struct dra7xx_pcie *dra7xx, >> return 0; >> } >> >> +static int dra7xx_pcie_reset(struct platform_device *pdev) >> +{ >> + int ret; >> + struct device *dev = &pdev->dev; >> + struct pci_dra7xx_platform_data *pdata = pdev->dev.platform_data; >> + >> + if (!(pdata && pdata->deassert_reset && pdata->assert_reset)) { >> + dev_err(dev, "platform data for reset not found!\n"); >> + return -EINVAL; >> + } >> + >> + ret = pdata->assert_reset(pdev, pdata->reset_name); >> + if (ret) { >> + dev_err(dev, "assert_reset failed: %d\n", ret); >> + return ret; >> + } >> + >> + ret = pdata->deassert_reset(pdev, pdata->reset_name); >> + if (ret) { >> + dev_err(dev, "deassert_reset failed: %d\n", ret); >> + return ret; >> + } > > The only comment I have on this is the symmetry (assert_reset invocation > in driver remove). If you install and remove the module once, then the > reset stays deasserted. On Power-On-Reset, the resets by default will be > in asserted state. hmm.. not sure of the benefits of leaving the reset lines de-asserted during remove. The idea is irrespective of the initial sate or power-on state, during probe the driver should assert and de-assert the reset lines. Thanks Kishon -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi, On Thursday 14 January 2016 02:07 PM, Kishon Vijay Abraham I wrote: > Hi Suman, > > On Wednesday 13 January 2016 11:21 PM, Suman Anna wrote: >> On 01/13/2016 12:47 AM, Kishon Vijay Abraham I wrote: >>> Use platform populated reset assert and deassert >>> callbacks to perform reset of PCIe. >>> >>> Use these callbacks until a reset interface using drivers/reset >>> is available for the purpose. >>> >>> Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com> >>> Signed-off-by: Sekhar Nori <nsekhar@ti.com> >>> --- >>> drivers/pci/host/pci-dra7xx.c | 32 ++++++++++++++++++++++++++++++++ >>> 1 file changed, 32 insertions(+) >>> >>> diff --git a/drivers/pci/host/pci-dra7xx.c b/drivers/pci/host/pci-dra7xx.c >>> index 8c36880..049083d 100644 >>> --- a/drivers/pci/host/pci-dra7xx.c >>> +++ b/drivers/pci/host/pci-dra7xx.c >>> @@ -25,6 +25,8 @@ >>> #include <linux/resource.h> >>> #include <linux/types.h> >>> >>> +#include <linux/platform_data/pci-dra7xx.h> >>> + >>> #include "pcie-designware.h" >>> >>> /* PCIe controller wrapper DRA7XX configuration registers */ >>> @@ -329,6 +331,32 @@ static int __init dra7xx_add_pcie_port(struct dra7xx_pcie *dra7xx, >>> return 0; >>> } >>> >>> +static int dra7xx_pcie_reset(struct platform_device *pdev) >>> +{ >>> + int ret; >>> + struct device *dev = &pdev->dev; >>> + struct pci_dra7xx_platform_data *pdata = pdev->dev.platform_data; >>> + >>> + if (!(pdata && pdata->deassert_reset && pdata->assert_reset)) { >>> + dev_err(dev, "platform data for reset not found!\n"); >>> + return -EINVAL; >>> + } >>> + >>> + ret = pdata->assert_reset(pdev, pdata->reset_name); >>> + if (ret) { >>> + dev_err(dev, "assert_reset failed: %d\n", ret); >>> + return ret; >>> + } >>> + >>> + ret = pdata->deassert_reset(pdev, pdata->reset_name); >>> + if (ret) { >>> + dev_err(dev, "deassert_reset failed: %d\n", ret); >>> + return ret; >>> + } >> >> The only comment I have on this is the symmetry (assert_reset invocation >> in driver remove). If you install and remove the module once, then the >> reset stays deasserted. On Power-On-Reset, the resets by default will be >> in asserted state. > > hmm.. not sure of the benefits of leaving the reset lines de-asserted during > remove. The idea is irrespective of the initial sate or power-on state, during > probe the driver should assert and de-assert the reset lines. Also right now the pci-dra7xx can't be inserted as a module. However since that might be added in the future, I'll add assert_reset in the remove path of this driver. Thanks Kishon -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/pci/host/pci-dra7xx.c b/drivers/pci/host/pci-dra7xx.c index 8c36880..049083d 100644 --- a/drivers/pci/host/pci-dra7xx.c +++ b/drivers/pci/host/pci-dra7xx.c @@ -25,6 +25,8 @@ #include <linux/resource.h> #include <linux/types.h> +#include <linux/platform_data/pci-dra7xx.h> + #include "pcie-designware.h" /* PCIe controller wrapper DRA7XX configuration registers */ @@ -329,6 +331,32 @@ static int __init dra7xx_add_pcie_port(struct dra7xx_pcie *dra7xx, return 0; } +static int dra7xx_pcie_reset(struct platform_device *pdev) +{ + int ret; + struct device *dev = &pdev->dev; + struct pci_dra7xx_platform_data *pdata = pdev->dev.platform_data; + + if (!(pdata && pdata->deassert_reset && pdata->assert_reset)) { + dev_err(dev, "platform data for reset not found!\n"); + return -EINVAL; + } + + ret = pdata->assert_reset(pdev, pdata->reset_name); + if (ret) { + dev_err(dev, "assert_reset failed: %d\n", ret); + return ret; + } + + ret = pdata->deassert_reset(pdev, pdata->reset_name); + if (ret) { + dev_err(dev, "deassert_reset failed: %d\n", ret); + return ret; + } + + return 0; +} + static int __init dra7xx_pcie_probe(struct platform_device *pdev) { u32 reg; @@ -347,6 +375,10 @@ static int __init dra7xx_pcie_probe(struct platform_device *pdev) enum of_gpio_flags flags; unsigned long gpio_flags; + ret = dra7xx_pcie_reset(pdev); + if (ret) + return ret; + dra7xx = devm_kzalloc(dev, sizeof(*dra7xx), GFP_KERNEL); if (!dra7xx) return -ENOMEM;