All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jisheng Zhang <jszhang@marvell.com>
To: Sekhar Nori <nsekhar@ti.com>
Cc: Murali Karicheri <m-karicheri2@ti.com>,
	Bjorn Helgaas <bhelgaas@google.com>, <linux-pci@vger.kernel.org>,
	<linux-kernel@vger.kernel.org>,
	<linux-arm-kernel@lists.infradead.org>
Subject: Re: [PATCH] PCI: keystone: fix crash with deferred probe
Date: Mon, 23 Nov 2015 19:27:17 +0800	[thread overview]
Message-ID: <20151123192717.4d4ddf06@xhacker> (raw)
In-Reply-To: <20151123192209.38e2c101@xhacker>

On Mon, 23 Nov 2015 19:22:09 +0800
Jisheng Zhang <jszhang@marvell.com> wrote:

> Dear Sekhar,
> 
> On Fri, 20 Nov 2015 16:47:02 +0530
> Sekhar Nori wrote:
> 
> > Drop __init annotations for functions called during
> > probe of keystone PCIe host.
> > 
> > This fixes kernel crashes seen because of init data
> > reclamation if the driver defer probes because of
> > phy unavailability.
> > 
> > While at it, drop the useless __refdata annotation
> > to driver structure. If its really needed, it should
> > be added back with a comment explaining why.
> > 
> > Signed-off-by: Sekhar Nori <nsekhar@ti.com>
> > ---
> >  drivers/pci/host/pci-keystone-dw.c | 2 +-
> >  drivers/pci/host/pci-keystone.c    | 6 +++---
> >  2 files changed, 4 insertions(+), 4 deletions(-)
> > 
> > diff --git a/drivers/pci/host/pci-keystone-dw.c b/drivers/pci/host/pci-keystone-dw.c
> > index ed34c9520a02..47ddbcce53ca 100644
> > --- a/drivers/pci/host/pci-keystone-dw.c
> > +++ b/drivers/pci/host/pci-keystone-dw.c
> > @@ -469,7 +469,7 @@ void ks_dw_pcie_initiate_link_train(struct keystone_pcie *ks_pcie)
> >   * and call dw_pcie_v3_65_host_init() API to initialize the Keystone
> >   * PCI host controller.
> >   */
> > -int __init ks_dw_pcie_host_init(struct keystone_pcie *ks_pcie,
> > +int ks_dw_pcie_host_init(struct keystone_pcie *ks_pcie,
> >  				struct device_node *msi_intc_np)  
> 
> when trying to add berlin pcie support and came up one patch
> 
> http://lists.infradead.org/pipermail/linux-arm-kernel/2015-November/384575.html
> 
> when build testing, I also realized this issue, but keystone relies on

I mean ks_dw_pcie_host_init() calls hook_fault_code()

> hook_fault_code() which is will be freed after boot, so your patch isn't
> completed.

And will introduce section mismatch warnings

> 
> And from the keystone driver implementation, it seems assume the driver won't
> be defer probed.
> 
> Thanks
> 
> >  {
> >  	struct pcie_port *pp = &ks_pcie->pp;
> > diff --git a/drivers/pci/host/pci-keystone.c b/drivers/pci/host/pci-keystone.c
> > index 0aa81bd3de12..593a58834c7d 100644
> > --- a/drivers/pci/host/pci-keystone.c
> > +++ b/drivers/pci/host/pci-keystone.c
> > @@ -291,7 +291,7 @@ static struct pcie_host_ops keystone_pcie_host_ops = {
> >  	.scan_bus = ks_dw_pcie_v3_65_scan_bus,
> >  };
> >  
> > -static int __init ks_add_pcie_port(struct keystone_pcie *ks_pcie,
> > +static int ks_add_pcie_port(struct keystone_pcie *ks_pcie,
> >  			 struct platform_device *pdev)
> >  {
> >  	struct pcie_port *pp = &ks_pcie->pp;
> > @@ -340,7 +340,7 @@ static int __exit ks_pcie_remove(struct platform_device *pdev)
> >  	return 0;
> >  }
> >  
> > -static int __init ks_pcie_probe(struct platform_device *pdev)
> > +static int ks_pcie_probe(struct platform_device *pdev)
> >  {
> >  	struct device *dev = &pdev->dev;
> >  	struct keystone_pcie *ks_pcie;
> > @@ -396,7 +396,7 @@ fail_clk:
> >  	return ret;
> >  }
> >  
> > -static struct platform_driver ks_pcie_driver __refdata = {
> > +static struct platform_driver ks_pcie_driver = {
> >  	.probe  = ks_pcie_probe,
> >  	.remove = __exit_p(ks_pcie_remove),
> >  	.driver = {  
> 


WARNING: multiple messages have this Message-ID (diff)
From: jszhang@marvell.com (Jisheng Zhang)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH] PCI: keystone: fix crash with deferred probe
Date: Mon, 23 Nov 2015 19:27:17 +0800	[thread overview]
Message-ID: <20151123192717.4d4ddf06@xhacker> (raw)
In-Reply-To: <20151123192209.38e2c101@xhacker>

On Mon, 23 Nov 2015 19:22:09 +0800
Jisheng Zhang <jszhang@marvell.com> wrote:

> Dear Sekhar,
> 
> On Fri, 20 Nov 2015 16:47:02 +0530
> Sekhar Nori wrote:
> 
> > Drop __init annotations for functions called during
> > probe of keystone PCIe host.
> > 
> > This fixes kernel crashes seen because of init data
> > reclamation if the driver defer probes because of
> > phy unavailability.
> > 
> > While at it, drop the useless __refdata annotation
> > to driver structure. If its really needed, it should
> > be added back with a comment explaining why.
> > 
> > Signed-off-by: Sekhar Nori <nsekhar@ti.com>
> > ---
> >  drivers/pci/host/pci-keystone-dw.c | 2 +-
> >  drivers/pci/host/pci-keystone.c    | 6 +++---
> >  2 files changed, 4 insertions(+), 4 deletions(-)
> > 
> > diff --git a/drivers/pci/host/pci-keystone-dw.c b/drivers/pci/host/pci-keystone-dw.c
> > index ed34c9520a02..47ddbcce53ca 100644
> > --- a/drivers/pci/host/pci-keystone-dw.c
> > +++ b/drivers/pci/host/pci-keystone-dw.c
> > @@ -469,7 +469,7 @@ void ks_dw_pcie_initiate_link_train(struct keystone_pcie *ks_pcie)
> >   * and call dw_pcie_v3_65_host_init() API to initialize the Keystone
> >   * PCI host controller.
> >   */
> > -int __init ks_dw_pcie_host_init(struct keystone_pcie *ks_pcie,
> > +int ks_dw_pcie_host_init(struct keystone_pcie *ks_pcie,
> >  				struct device_node *msi_intc_np)  
> 
> when trying to add berlin pcie support and came up one patch
> 
> http://lists.infradead.org/pipermail/linux-arm-kernel/2015-November/384575.html
> 
> when build testing, I also realized this issue, but keystone relies on

I mean ks_dw_pcie_host_init() calls hook_fault_code()

> hook_fault_code() which is will be freed after boot, so your patch isn't
> completed.

And will introduce section mismatch warnings

> 
> And from the keystone driver implementation, it seems assume the driver won't
> be defer probed.
> 
> Thanks
> 
> >  {
> >  	struct pcie_port *pp = &ks_pcie->pp;
> > diff --git a/drivers/pci/host/pci-keystone.c b/drivers/pci/host/pci-keystone.c
> > index 0aa81bd3de12..593a58834c7d 100644
> > --- a/drivers/pci/host/pci-keystone.c
> > +++ b/drivers/pci/host/pci-keystone.c
> > @@ -291,7 +291,7 @@ static struct pcie_host_ops keystone_pcie_host_ops = {
> >  	.scan_bus = ks_dw_pcie_v3_65_scan_bus,
> >  };
> >  
> > -static int __init ks_add_pcie_port(struct keystone_pcie *ks_pcie,
> > +static int ks_add_pcie_port(struct keystone_pcie *ks_pcie,
> >  			 struct platform_device *pdev)
> >  {
> >  	struct pcie_port *pp = &ks_pcie->pp;
> > @@ -340,7 +340,7 @@ static int __exit ks_pcie_remove(struct platform_device *pdev)
> >  	return 0;
> >  }
> >  
> > -static int __init ks_pcie_probe(struct platform_device *pdev)
> > +static int ks_pcie_probe(struct platform_device *pdev)
> >  {
> >  	struct device *dev = &pdev->dev;
> >  	struct keystone_pcie *ks_pcie;
> > @@ -396,7 +396,7 @@ fail_clk:
> >  	return ret;
> >  }
> >  
> > -static struct platform_driver ks_pcie_driver __refdata = {
> > +static struct platform_driver ks_pcie_driver = {
> >  	.probe  = ks_pcie_probe,
> >  	.remove = __exit_p(ks_pcie_remove),
> >  	.driver = {  
> 

  reply	other threads:[~2015-11-23 11:27 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-11-20 11:17 [PATCH] PCI: keystone: fix crash with deferred probe Sekhar Nori
2015-11-20 11:17 ` Sekhar Nori
2015-11-23 11:22 ` Jisheng Zhang
2015-11-23 11:22   ` Jisheng Zhang
2015-11-23 11:27   ` Jisheng Zhang [this message]
2015-11-23 11:27     ` Jisheng Zhang
2015-11-23 16:48     ` Sekhar Nori
2015-11-23 16:48       ` Sekhar Nori
2015-11-23 11:39   ` Russell King - ARM Linux
2015-11-23 11:39     ` Russell King - ARM Linux

Reply instructions:

You may reply publicly 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=20151123192717.4d4ddf06@xhacker \
    --to=jszhang@marvell.com \
    --cc=bhelgaas@google.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pci@vger.kernel.org \
    --cc=m-karicheri2@ti.com \
    --cc=nsekhar@ti.com \
    /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
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.