[PATCH] PCI: xgene: fix a mistake about cfg address
It has a wrong modification to the xgene driver by the commit
e2dcd20b1645a. it use devm_platform_ioremap_resource_byname() to
simplify codes and remove the res variable, But the following code
needs to use this res variable, So after this commit, the port->cfg_addr
will get a wrong address. Now, revert it.
Fixes: e2dcd20b1645a ("PCI: controller: Convert to
devm_platform_ioremap_resource_byname()")
Reported-by: dann.fraz...@canonical.com
Signed-off-by: Dejin Zheng
---
drivers/pci/controller/pci-xgene.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/pci/controller/pci-xgene.c
b/drivers/pci/controller/pci-xgene.c
index 2afdc865253e..7f503dd4ff81 100644
--- a/drivers/pci/controller/pci-xgene.c
+++ b/drivers/pci/controller/pci-xgene.c
@@ -354,7 +354,8 @@ static int xgene_pcie_map_reg(struct xgene_pcie_port *port,
if (IS_ERR(port->csr_base))
return PTR_ERR(port->csr_base);
- port->cfg_base = devm_platform_ioremap_resource_byname(pdev, "cfg");
+ res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "cfg");
+ port->cfg_base = devm_ioremap_resource(dev, res);
if (IS_ERR(port->cfg_base))
return PTR_ERR(port->cfg_base);
port->cfg_addr = res->start;
--
2.30.1
Re: [PATCH v2] PCI: controller: convert to devm_platform_ioremap_resource_byname()
On Sat, Mar 27, 2021 at 12:02:42PM -0600, dann frazier wrote: Hi Dann, I'm so sorry for that, And there is a mistake with my patch that caused this problem. Thank you very much for telling me this, I will fix it as soon as possible. > On Wed, Jun 03, 2020 at 01:16:01AM +0800, Dejin Zheng wrote: > > Use devm_platform_ioremap_resource_byname() to simplify codes. > > it contains platform_get_resource_byname() and devm_ioremap_resource(). > > > > Signed-off-by: Dejin Zheng > > --- > > v1 -> v2: > > - Discard changes to the file drivers/pci/controller/pcie-xilinx-nwl.c > > Due to my mistakes, my patch will modify pcie-xilinx-nwl.c, > > but it still need to use the res variable, but > > devm_platform_ioremap_resource_byname() funtion can't assign a > > value to the variable res. kbuild test robot report it. Thanks > > very much for kbuild test robot . > > > > drivers/pci/controller/cadence/pcie-cadence-ep.c | 3 +-- > > drivers/pci/controller/cadence/pcie-cadence-host.c | 3 +-- > > drivers/pci/controller/pci-tegra.c | 8 +++- > > drivers/pci/controller/pci-xgene.c | 3 +-- > > drivers/pci/controller/pcie-altera-msi.c | 3 +-- > > drivers/pci/controller/pcie-altera.c | 9 +++-- > > drivers/pci/controller/pcie-mediatek.c | 4 +--- > > drivers/pci/controller/pcie-rockchip.c | 5 ++--- > > 8 files changed, 13 insertions(+), 25 deletions(-) > > > > hey, > I found that recent kernels fail to initialize PCI devices on our HP > m400 Moonshot cartridges, which are based on the X-Gene SoC. I > bisected the issue down to this commit. I found that just reverting > this hunk in pci-xgene.c is enough to get v5.12 rcs booting again: > > > diff --git a/drivers/pci/controller/pci-xgene.c > > b/drivers/pci/controller/pci-xgene.c > > index d1efa8ffbae1..1431a18eb02c 100644 > > --- a/drivers/pci/controller/pci-xgene.c > > +++ b/drivers/pci/controller/pci-xgene.c > > @@ -355,8 +355,7 @@ static int xgene_pcie_map_reg(struct xgene_pcie_port > > *port, > > if (IS_ERR(port->csr_base)) > > return PTR_ERR(port->csr_base); > > > > - res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "cfg"); > > - port->cfg_base = devm_ioremap_resource(dev, res); > > + port->cfg_base = devm_platform_ioremap_resource_byname(pdev, "cfg"); > > if (IS_ERR(port->cfg_base)) > > return PTR_ERR(port->cfg_base); > > port->cfg_addr = res->start; The mistake of this patch is here, port->cfg_addr need the res->start, But this patch remove the res of get "cfg" resource. here use the wrong data by get "csr" resource in the previous. BR, Dejin > > > In case it helps, here's the PCI initialization portion of dmesg when > it fails: > > [0.756359] xgene-pcie 1f50.pcie: host bridge /soc/pcie@1f50 > ranges: > [0.756372] xgene-pcie 1f50.pcie: No bus range found for > /soc/pcie@1f50, using [bus 00-ff] > [0.756387] xgene-pcie 1f50.pcie: MEM 0xa13000..0xa1afff > -> 0x003000 > [0.756404] xgene-pcie 1f50.pcie: IB MEM 0x40..0x7f > -> 0x40 > [0.756459] xgene-pcie 1f50.pcie: (rc) x8 gen-2 link up > [0.756525] xgene-pcie 1f50.pcie: PCI host bridge to bus :00 > [0.756532] pci_bus :00: root bus resource [bus 00-ff] > [0.756538] pci_bus :00: root bus resource [mem > 0xa13000-0xa1afff] (bus address [0x3000-0xafff]) > > > and here's what it looks like when it works: > > [0.756793] xgene-pcie 1f50.pcie: host bridge /soc/pcie@1f50 > ranges: > [0.756807] xgene-pcie 1f50.pcie: No bus range found for > /soc/pcie@1f50, using [bus 00-ff] > [0.756822] xgene-pcie 1f50.pcie: MEM 0xa13000..0xa1afff > -> 0x003000 > [0.756838] xgene-pcie 1f50.pcie: IB MEM 0x40..0x7f > -> 0x40 > [0.756892] xgene-pcie 1f50.pcie: (rc) x8 gen-2 link up > [0.756962] xgene-pcie 1f50.pcie: PCI host bridge to bus :00 > [0.756968] pci_bus :00: root bus resource [bus 00-ff] > [0.756974] pci_bus :00: root bus resource [mem > 0xa13000-0xa1afff] (bus address [0x3000-0xafff]) > [0.757006] pci :00:00.0: [10e8:e004] type 01 class 0x060400 > [0.757014] pci_bus :00: 2-byte config write to :00:00.0 offset > 0x4 may corrupt adjacent RW1C bits > [0.757022] pci_bus :00: 2-byte config write
Re: [PATCH v5 0/4] Introduce pcim_alloc_irq_vectors()
On Fri, Feb 26, 2021 at 08:20:55PM +0100, Robert Richter wrote: > On 26.02.21 23:50:52, Dejin Zheng wrote: > > Introduce pcim_alloc_irq_vectors(), a device-managed version of > > pci_alloc_irq_vectors(), In some i2c drivers, If pcim_enable_device() > > has been called before, then pci_alloc_irq_vectors() is actually a > > device-managed function. It is used as a device-managed function, So > > replace it with pcim_alloc_irq_vectors(). > > For the whole series: > > Reviewed-by: Robert Richter > Robert, Thanks very much for your help! > Thanks.
Re: [PATCH v5 1/4] PCI: Introduce pcim_alloc_irq_vectors()
On Fri, Feb 26, 2021 at 06:23:02PM +0200, Andy Shevchenko wrote:
> On Fri, Feb 26, 2021 at 11:50:53PM +0800, Dejin Zheng wrote:
> > Introduce pcim_alloc_irq_vectors(), a device-managed version of
> > pci_alloc_irq_vectors(). Introducing this function can simplify
> > the error handling path in many drivers.
> >
> > And use pci_free_irq_vectors() to replace some code in pcim_release(),
> > they are equivalent, and no functional change. It is more explicit
> > that pcim_alloc_irq_vectors() is a device-managed function.
>
> Thanks!
>
> Reviewed-by: Andy Shevchenko
>
Andy, Thanks!
> > Suggested-by: Andy Shevchenko
> > Signed-off-by: Dejin Zheng
> > ---
> > v4 -> v5:
> > - Remove the check of enable device in pcim_alloc_irq_vectors()
> > and make it as a static line function.
> > v3 -> v4:
> > - No change
> > v2 -> v3:
> > - Add some commit comments for replace some codes in
> > pcim_release() by pci_free_irq_vectors().
> > v1 -> v2:
> > - Use pci_free_irq_vectors() to replace some code in
> > pcim_release().
> > - Modify some commit messages.
> >
> > drivers/pci/pci.c | 5 +
> > include/linux/pci.h | 24
> > 2 files changed, 25 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> > index 16a17215f633..fecfdc0add2f 100644
> > --- a/drivers/pci/pci.c
> > +++ b/drivers/pci/pci.c
> > @@ -1969,10 +1969,7 @@ static void pcim_release(struct device *gendev, void
> > *res)
> > struct pci_devres *this = res;
> > int i;
> >
> > - if (dev->msi_enabled)
> > - pci_disable_msi(dev);
> > - if (dev->msix_enabled)
> > - pci_disable_msix(dev);
> > + pci_free_irq_vectors(dev);
> >
> > for (i = 0; i < DEVICE_COUNT_RESOURCE; i++)
> > if (this->region_mask & (1 << i))
> > diff --git a/include/linux/pci.h b/include/linux/pci.h
> > index 86c799c97b77..5cafd7d65fd7 100644
> > --- a/include/linux/pci.h
> > +++ b/include/linux/pci.h
> > @@ -1818,6 +1818,30 @@ pci_alloc_irq_vectors(struct pci_dev *dev, unsigned
> > int min_vecs,
> > NULL);
> > }
> >
> > +/**
> > + * pcim_alloc_irq_vectors - a device-managed pci_alloc_irq_vectors()
> > + * @dev: PCI device to operate on
> > + * @min_vecs: minimum number of vectors required (must be >=
> > 1)
> > + * @max_vecs: maximum (desired) number of vectors
> > + * @flags: flags or quirks for the allocation
> > + *
> > + * Return the number of vectors allocated, (which might be smaller than
> > + * @max_vecs) if successful, or a negative error code on error. If less
> > + * than @min_vecs interrupt vectors are available for @dev the function
> > + * will fail with -ENOSPC.
> > + *
> > + * It depends on calling pcim_enable_device() to make IRQ resources
> > + * manageable.
> > + */
> > +static inline int
> > +pcim_alloc_irq_vectors(struct pci_dev *dev, unsigned int min_vecs,
> > + unsigned int max_vecs, unsigned int flags)
> > +{
> > + if (!pci_is_managed(dev))
> > + return -EINVAL;
> > + return pci_alloc_irq_vectors(dev, min_vecs, max_vecs, flags);
> > +}
> > +
> > /* Include architecture-dependent settings and functions */
> >
> > #include
> > --
> > 2.25.0
> >
>
> --
> With Best Regards,
> Andy Shevchenko
>
>
[PATCH v5 4/4] i2c: thunderx: Use pcim_alloc_irq_vectors() to allocate IRQ vectors
The pcim_alloc_irq_vectors() function, an explicit device-managed version of pci_alloc_irq_vectors(). If pcim_enable_device() has been called before, then pci_alloc_irq_vectors() is actually a device-managed function. It is used here as a device-managed function, So replace it with pcim_alloc_irq_vectors(). Signed-off-by: Dejin Zheng --- v4 -> v5: - Modify the subject name. v3 -> v4: - No change. v2 -> v3: - No change. v1 -> v2: - Modify some commit messages. drivers/i2c/busses/i2c-thunderx-pcidrv.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/i2c/busses/i2c-thunderx-pcidrv.c b/drivers/i2c/busses/i2c-thunderx-pcidrv.c index 12c90aa0900e..63354e9fb726 100644 --- a/drivers/i2c/busses/i2c-thunderx-pcidrv.c +++ b/drivers/i2c/busses/i2c-thunderx-pcidrv.c @@ -192,7 +192,7 @@ static int thunder_i2c_probe_pci(struct pci_dev *pdev, i2c->hlc_int_enable = thunder_i2c_hlc_int_enable; i2c->hlc_int_disable = thunder_i2c_hlc_int_disable; - ret = pci_alloc_irq_vectors(pdev, 1, 1, PCI_IRQ_MSIX); + ret = pcim_alloc_irq_vectors(pdev, 1, 1, PCI_IRQ_MSIX); if (ret < 0) goto error; -- 2.25.0
[PATCH v5 2/4] Documentation: devres: Add pcim_alloc_irq_vectors()
Add pcim_alloc_irq_vectors(), a device-managed version of pci_alloc_irq_vectors(). introducing this function can simplify the error handling path in many drivers. Signed-off-by: Dejin Zheng --- v4 -> v5: - No change v3 -> v4: - No change v2 -> v3: - No change v1 -> v2: - Modify some commit messages. Documentation/driver-api/driver-model/devres.rst | 1 + 1 file changed, 1 insertion(+) diff --git a/Documentation/driver-api/driver-model/devres.rst b/Documentation/driver-api/driver-model/devres.rst index cd8b6e657b94..a52f65b6352f 100644 --- a/Documentation/driver-api/driver-model/devres.rst +++ b/Documentation/driver-api/driver-model/devres.rst @@ -380,6 +380,7 @@ PCI devm_pci_alloc_host_bridge() : managed PCI host bridge allocation devm_pci_remap_cfgspace(): ioremap PCI configuration space devm_pci_remap_cfg_resource(): ioremap PCI configuration space resource + pcim_alloc_irq_vectors() : managed IRQ vectors allocation pcim_enable_device() : after success, all PCI ops become managed pcim_pin_device(): keep PCI device enabled after release -- 2.25.0
[PATCH v5 3/4] i2c: designware: Use pcim_alloc_irq_vectors() to allocate IRQ vectors
The pcim_alloc_irq_vectors() function, an explicit device-managed version
of pci_alloc_irq_vectors(). If pcim_enable_device() has been called
before, then pci_alloc_irq_vectors() is actually a device-managed
function. It is used here as a device-managed function, So replace it
with pcim_alloc_irq_vectors(). At the same time, Remove the
pci_free_irq_vectors() function to simplify the error handling path.
the freeing resources will take automatically when device is gone.
Acked-by: Andy Shevchenko
Signed-off-by: Dejin Zheng
---
v4 -> v5:
- Modify the subject name.
v3 -> v4:
- add some commit comments.
v2 -> v3:
- simplify the error handling path.
v1 -> v2:
- Modify some commit messages.
drivers/i2c/busses/i2c-designware-pcidrv.c | 15 ---
1 file changed, 4 insertions(+), 11 deletions(-)
diff --git a/drivers/i2c/busses/i2c-designware-pcidrv.c
b/drivers/i2c/busses/i2c-designware-pcidrv.c
index 55c83a7a24f3..620b41e373b6 100644
--- a/drivers/i2c/busses/i2c-designware-pcidrv.c
+++ b/drivers/i2c/busses/i2c-designware-pcidrv.c
@@ -219,7 +219,7 @@ static int i2c_dw_pci_probe(struct pci_dev *pdev,
if (!dev)
return -ENOMEM;
- r = pci_alloc_irq_vectors(pdev, 1, 1, PCI_IRQ_ALL_TYPES);
+ r = pcim_alloc_irq_vectors(pdev, 1, 1, PCI_IRQ_ALL_TYPES);
if (r < 0)
return r;
@@ -234,10 +234,8 @@ static int i2c_dw_pci_probe(struct pci_dev *pdev,
if (controller->setup) {
r = controller->setup(pdev, controller);
- if (r) {
- pci_free_irq_vectors(pdev);
+ if (r)
return r;
- }
}
i2c_dw_adjust_bus_speed(dev);
@@ -246,10 +244,8 @@ static int i2c_dw_pci_probe(struct pci_dev *pdev,
i2c_dw_acpi_configure(>dev);
r = i2c_dw_validate_speed(dev);
- if (r) {
- pci_free_irq_vectors(pdev);
+ if (r)
return r;
- }
i2c_dw_configure(dev);
@@ -269,10 +265,8 @@ static int i2c_dw_pci_probe(struct pci_dev *pdev,
adap->nr = controller->bus_num;
r = i2c_dw_probe(dev);
- if (r) {
- pci_free_irq_vectors(pdev);
+ if (r)
return r;
- }
pm_runtime_set_autosuspend_delay(>dev, 1000);
pm_runtime_use_autosuspend(>dev);
@@ -292,7 +286,6 @@ static void i2c_dw_pci_remove(struct pci_dev *pdev)
i2c_del_adapter(>adapter);
devm_free_irq(>dev, dev->irq, dev);
- pci_free_irq_vectors(pdev);
}
/* work with hotplug and coldplug */
--
2.25.0
[PATCH v5 1/4] PCI: Introduce pcim_alloc_irq_vectors()
Introduce pcim_alloc_irq_vectors(), a device-managed version of
pci_alloc_irq_vectors(). Introducing this function can simplify
the error handling path in many drivers.
And use pci_free_irq_vectors() to replace some code in pcim_release(),
they are equivalent, and no functional change. It is more explicit
that pcim_alloc_irq_vectors() is a device-managed function.
Suggested-by: Andy Shevchenko
Signed-off-by: Dejin Zheng
---
v4 -> v5:
- Remove the check of enable device in pcim_alloc_irq_vectors()
and make it as a static line function.
v3 -> v4:
- No change
v2 -> v3:
- Add some commit comments for replace some codes in
pcim_release() by pci_free_irq_vectors().
v1 -> v2:
- Use pci_free_irq_vectors() to replace some code in
pcim_release().
- Modify some commit messages.
drivers/pci/pci.c | 5 +
include/linux/pci.h | 24
2 files changed, 25 insertions(+), 4 deletions(-)
diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index 16a17215f633..fecfdc0add2f 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -1969,10 +1969,7 @@ static void pcim_release(struct device *gendev, void
*res)
struct pci_devres *this = res;
int i;
- if (dev->msi_enabled)
- pci_disable_msi(dev);
- if (dev->msix_enabled)
- pci_disable_msix(dev);
+ pci_free_irq_vectors(dev);
for (i = 0; i < DEVICE_COUNT_RESOURCE; i++)
if (this->region_mask & (1 << i))
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 86c799c97b77..5cafd7d65fd7 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -1818,6 +1818,30 @@ pci_alloc_irq_vectors(struct pci_dev *dev, unsigned int
min_vecs,
NULL);
}
+/**
+ * pcim_alloc_irq_vectors - a device-managed pci_alloc_irq_vectors()
+ * @dev: PCI device to operate on
+ * @min_vecs: minimum number of vectors required (must be >= 1)
+ * @max_vecs: maximum (desired) number of vectors
+ * @flags: flags or quirks for the allocation
+ *
+ * Return the number of vectors allocated, (which might be smaller than
+ * @max_vecs) if successful, or a negative error code on error. If less
+ * than @min_vecs interrupt vectors are available for @dev the function
+ * will fail with -ENOSPC.
+ *
+ * It depends on calling pcim_enable_device() to make IRQ resources
+ * manageable.
+ */
+static inline int
+pcim_alloc_irq_vectors(struct pci_dev *dev, unsigned int min_vecs,
+ unsigned int max_vecs, unsigned int flags)
+{
+ if (!pci_is_managed(dev))
+ return -EINVAL;
+ return pci_alloc_irq_vectors(dev, min_vecs, max_vecs, flags);
+}
+
/* Include architecture-dependent settings and functions */
#include
--
2.25.0
[PATCH v5 0/4] Introduce pcim_alloc_irq_vectors()
Introduce pcim_alloc_irq_vectors(), a device-managed version of pci_alloc_irq_vectors(), In some i2c drivers, If pcim_enable_device() has been called before, then pci_alloc_irq_vectors() is actually a device-managed function. It is used as a device-managed function, So replace it with pcim_alloc_irq_vectors(). Changelog - v4 -> v5: - Remove the check of enable device in pcim_alloc_irq_vectors() and make it as a static line function. - Modify the subject name in patch 3 and patch 4. v3 -> v4: - add some commit comments for patch 3 v2 -> v3: - Add some commit comments for replace some codes in pcim_release() by pci_free_irq_vectors(). - Simplify the error handling path in i2c designware driver. v1 -> v2: - Use pci_free_irq_vectors() to replace some code in pcim_release(). - Modify some commit messages. Dejin Zheng (4): PCI: Introduce pcim_alloc_irq_vectors() Documentation: devres: Add pcim_alloc_irq_vectors() i2c: designware: Use pcim_alloc_irq_vectors() to allocate IRQ vectors i2c: thunderx: Use pcim_alloc_irq_vectors() to allocate IRQ vectors .../driver-api/driver-model/devres.rst| 1 + drivers/i2c/busses/i2c-designware-pcidrv.c| 15 drivers/i2c/busses/i2c-thunderx-pcidrv.c | 2 +- drivers/pci/pci.c | 5 +--- include/linux/pci.h | 24 +++ 5 files changed, 31 insertions(+), 16 deletions(-) -- 2.25.0
Re: [PATCH v4 1/4] PCI: Introduce pcim_alloc_irq_vectors()
On Thu, Feb 25, 2021 at 10:33:53AM +0100, Robert Richter wrote: > On 23.02.21 22:14:35, Dejin Zheng wrote: > > On Tue, Feb 23, 2021 at 09:02:54AM +0100, Robert Richter wrote: > > > On 22.02.21 23:14:15, Dejin Zheng wrote: > > > > On Mon, Feb 22, 2021 at 11:56:08AM +0100, Robert Richter wrote: > > > > > On 20.02.21 00:46:49, Dejin Zheng wrote: > > > > > > > On 18.02.21 23:04:55, Dejin Zheng wrote: > > > > > > > > > > > > > + if (!dr || !dr->enabled) > > > > > > here checks whether the pci device is enabled. > > > > > > > > > > What is the purpose of this? The device "is_managed" or not. > > > > > > > > > The device is managed or not by check whether "dr" is NULL. And > > > > check the "dr->enabled" is for the PCI device enable. I think it > > > > may not make sense to apply for irq vectors when PCI device is not > > > > enabled. > > > > > > I don't see how a disabled device affects in any way the release of > > > the irq vectors during device removal. dr is always non-null in case > > > the device is managed, a check isn't needed for that. > > > > > Yes, the disabled device does not affect release irq vectors, But > > the disabled device affects apply for irq vectors, It is wrong to apply > > for the irq vectors when the device is not enabled. > > What is the scenario you have in mind here? What does happen then? > The typical use case is to pcim_enable_device() it and then add the > irq vectors. It is always enabled then. > > Even if the device could wrongly be disabled, it does not affect the > device's release. > > Also, how is this related to pcim? There isn't a check in > pci_alloc_irq_vectors() either for that case. > > > Add this check can > > facilitate developers to find problems as soon as possible. > > No, there are many ways to shoot yourself in the foot. We cannot add > checks here and there for this, esp. at runtime. If there is a valid > reason that the device must always be enabled and we cannot assume > this is the case, then we could add a WARN_ON(). But I doubt that. > Robert, You are right, I will remove the enable check. Thanks! BR, Dejin > -Robert
Re: [PATCH v4 1/4] PCI: Introduce pcim_alloc_irq_vectors()
On Tue, Feb 23, 2021 at 09:02:54AM +0100, Robert Richter wrote: > On 22.02.21 23:14:15, Dejin Zheng wrote: > > On Mon, Feb 22, 2021 at 11:56:08AM +0100, Robert Richter wrote: > > > On 20.02.21 00:46:49, Dejin Zheng wrote: > > > > > On 18.02.21 23:04:55, Dejin Zheng wrote: > > > > > > > > > + if (!dr || !dr->enabled) > > > > here checks whether the pci device is enabled. > > > > > > What is the purpose of this? The device "is_managed" or not. > > > > > The device is managed or not by check whether "dr" is NULL. And > > check the "dr->enabled" is for the PCI device enable. I think it > > may not make sense to apply for irq vectors when PCI device is not > > enabled. > > I don't see how a disabled device affects in any way the release of > the irq vectors during device removal. dr is always non-null in case > the device is managed, a check isn't needed for that. > Yes, the disabled device does not affect release irq vectors, But the disabled device affects apply for irq vectors, It is wrong to apply for the irq vectors when the device is not enabled. Add this check can facilitate developers to find problems as soon as possible. > -Robert
Re: [PATCH v4 1/4] PCI: Introduce pcim_alloc_irq_vectors()
On Mon, Feb 22, 2021 at 11:56:08AM +0100, Robert Richter wrote: > On 20.02.21 00:46:49, Dejin Zheng wrote: > > > On 18.02.21 23:04:55, Dejin Zheng wrote: > > > > > + if (!dr || !dr->enabled) > > here checks whether the pci device is enabled. > > What is the purpose of this? The device "is_managed" or not. > The device is managed or not by check whether "dr" is NULL. And check the "dr->enabled" is for the PCI device enable. I think it may not make sense to apply for irq vectors when PCI device is not enabled. PCI device enable by call pci_enable_device() function, this function initialize device before it's used by a driver. Ask low-level code to enable I/O and memory. Wake up the device if it was suspended. So I think it might be better to return to failure when it is found the PCI device is not enabled in the pcim_alloc_irq_vectors() function. It can facilitate developers to find problems as soon as possible. BR, Dejin > -Robert
Re: [PATCH v4 1/4] PCI: Introduce pcim_alloc_irq_vectors()
On Fri, Feb 19, 2021 at 03:40:05PM +0100, Robert Richter wrote:
> On 18.02.21 23:04:55, Dejin Zheng wrote:
> > Introduce pcim_alloc_irq_vectors(), a device-managed version of
> > pci_alloc_irq_vectors(). Introducing this function can simplify
> > the error handling path in many drivers.
> >
> > And use pci_free_irq_vectors() to replace some code in pcim_release(),
> > they are equivalent, and no functional change. It is more explicit
> > that pcim_alloc_irq_vectors() is a device-managed function.
> >
> > Suggested-by: Andy Shevchenko
> > Signed-off-by: Dejin Zheng
> > ---
> > v3 -> v4:
> > - No change
> > v2 -> v3:
> > - Add some commit comments for replace some codes in
> > pcim_release() by pci_free_irq_vectors().
> > v1 -> v2:
> > - Use pci_free_irq_vectors() to replace some code in
> > pcim_release().
> > - Modify some commit messages.
> >
> > drivers/pci/pci.c | 33 +
> > include/linux/pci.h | 3 +++
> > 2 files changed, 32 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> > index b67c4327d307..db799d089c85 100644
> > --- a/drivers/pci/pci.c
> > +++ b/drivers/pci/pci.c
> > @@ -1969,10 +1969,7 @@ static void pcim_release(struct device *gendev, void
> > *res)
> > struct pci_devres *this = res;
> > int i;
> >
> > - if (dev->msi_enabled)
> > - pci_disable_msi(dev);
> > - if (dev->msix_enabled)
> > - pci_disable_msix(dev);
> > + pci_free_irq_vectors(dev);
> >
> > for (i = 0; i < DEVICE_COUNT_RESOURCE; i++)
> > if (this->region_mask & (1 << i))
> > @@ -2054,6 +2051,34 @@ void pcim_pin_device(struct pci_dev *pdev)
> > }
> > EXPORT_SYMBOL(pcim_pin_device);
> >
> > +/**
> > + * pcim_alloc_irq_vectors - a device-managed pci_alloc_irq_vectors()
> > + * @dev: PCI device to operate on
> > + * @min_vecs: minimum number of vectors required (must be >=
> > 1)
> > + * @max_vecs: maximum (desired) number of vectors
> > + * @flags: flags or quirks for the allocation
> > + *
> > + * Return the number of vectors allocated, (which might be smaller than
> > + * @max_vecs) if successful, or a negative error code on error. If less
> > + * than @min_vecs interrupt vectors are available for @dev the function
> > + * will fail with -ENOSPC.
> > + *
> > + * It depends on calling pcim_enable_device() to make IRQ resources
> > + * manageable.
> > + */
> > +int pcim_alloc_irq_vectors(struct pci_dev *dev, unsigned int min_vecs,
> > + unsigned int max_vecs, unsigned int flags)
> > +{
> > + struct pci_devres *dr;
> > +
> > + dr = find_pci_dr(dev);
> > + if (!dr || !dr->enabled)
> > + return -EINVAL;
> > +
> > + return pci_alloc_irq_vectors(dev, min_vecs, max_vecs, flags);
> > +}
> > +EXPORT_SYMBOL(pcim_alloc_irq_vectors);
>
> If it is just about having a pcim-* counterpart why not just an inline
> function like the one below.
>
Robert and Andy,
First of all, thank you very much for your suggestions and help.
I think this is not just a pcim-* counterpart, I may not explain this
place clearly. In addition to calling pci_alloc_irq_vectors(), the
pcim_alloc_irq_vectors() function also checks whether the pci device is
enabled and whether the pci device resource has been managed. If any one
is wrong, it will return failure. Therefore, I think it should be used
as a function. For novices, maybe I understand it incorrectly, so I look
forward to your suggestions.
> > + dr = find_pci_dr(dev);
static struct pci_devres *find_pci_dr(struct pci_dev *pdev)
{
if (pci_is_managed(pdev))
return devres_find(>dev, pcim_release, NULL, NULL);
return NULL;
}
here checks whether the pci device resource has been managed.
> > + if (!dr || !dr->enabled)
here checks whether the pci device is enabled.
int pcim_enable_device(struct pci_dev *pdev)
{
struct pci_devres *dr;
int rc;
dr = get_pci_dr(pdev);
if (unlikely(!dr))
return -ENOMEM;
if (dr->enabled)
return 0;
rc = pci_enable_device(pdev);
if (!rc) {
pdev->is_managed = 1;
dr->enabled = 1;
}
return rc;
}
BR,
Dejin
> > +
> > /*
> > * pcibios_add_device - provide arch specific hook
[PATCH v1] Documentation: devres: add missing entry for pcim_set_mwi()
The pcim_set_mwi() should be documented in devres.rst. So add the missing entry. Signed-off-by: Dejin Zheng --- Documentation/driver-api/driver-model/devres.rst | 1 + 1 file changed, 1 insertion(+) diff --git a/Documentation/driver-api/driver-model/devres.rst b/Documentation/driver-api/driver-model/devres.rst index cd8b6e657b94..164c9cddc6d2 100644 --- a/Documentation/driver-api/driver-model/devres.rst +++ b/Documentation/driver-api/driver-model/devres.rst @@ -382,6 +382,7 @@ PCI devm_pci_remap_cfg_resource(): ioremap PCI configuration space resource pcim_enable_device() : after success, all PCI ops become managed pcim_pin_device(): keep PCI device enabled after release + pcim_set_mwi(): enables memory-write-invalidate PCI transaction PHY devm_usb_get_phy() -- 2.25.0
[PATCH v4 3/4] i2c: designware: Use the correct name of device-managed function
Use the new function pcim_alloc_irq_vectors() to allocate IRQ vectors,
the pcim_alloc_irq_vectors() function, an explicit device-managed version
of pci_alloc_irq_vectors(). If pcim_enable_device() has been called
before, then pci_alloc_irq_vectors() is actually a device-managed
function. It is used here as a device-managed function, So replace it
with pcim_alloc_irq_vectors(). At the same time, Remove the
pci_free_irq_vectors() function to simplify the error handling path.
the freeing resources will take automatically when device is gone.
Signed-off-by: Dejin Zheng
---
v3 -> v4:
- add some commit comments.
v2 -> v3:
- simplify the error handling path.
v1 -> v2:
- Modify some commit messages.
drivers/i2c/busses/i2c-designware-pcidrv.c | 15 ---
1 file changed, 4 insertions(+), 11 deletions(-)
diff --git a/drivers/i2c/busses/i2c-designware-pcidrv.c
b/drivers/i2c/busses/i2c-designware-pcidrv.c
index 55c83a7a24f3..620b41e373b6 100644
--- a/drivers/i2c/busses/i2c-designware-pcidrv.c
+++ b/drivers/i2c/busses/i2c-designware-pcidrv.c
@@ -219,7 +219,7 @@ static int i2c_dw_pci_probe(struct pci_dev *pdev,
if (!dev)
return -ENOMEM;
- r = pci_alloc_irq_vectors(pdev, 1, 1, PCI_IRQ_ALL_TYPES);
+ r = pcim_alloc_irq_vectors(pdev, 1, 1, PCI_IRQ_ALL_TYPES);
if (r < 0)
return r;
@@ -234,10 +234,8 @@ static int i2c_dw_pci_probe(struct pci_dev *pdev,
if (controller->setup) {
r = controller->setup(pdev, controller);
- if (r) {
- pci_free_irq_vectors(pdev);
+ if (r)
return r;
- }
}
i2c_dw_adjust_bus_speed(dev);
@@ -246,10 +244,8 @@ static int i2c_dw_pci_probe(struct pci_dev *pdev,
i2c_dw_acpi_configure(>dev);
r = i2c_dw_validate_speed(dev);
- if (r) {
- pci_free_irq_vectors(pdev);
+ if (r)
return r;
- }
i2c_dw_configure(dev);
@@ -269,10 +265,8 @@ static int i2c_dw_pci_probe(struct pci_dev *pdev,
adap->nr = controller->bus_num;
r = i2c_dw_probe(dev);
- if (r) {
- pci_free_irq_vectors(pdev);
+ if (r)
return r;
- }
pm_runtime_set_autosuspend_delay(>dev, 1000);
pm_runtime_use_autosuspend(>dev);
@@ -292,7 +286,6 @@ static void i2c_dw_pci_remove(struct pci_dev *pdev)
i2c_del_adapter(>adapter);
devm_free_irq(>dev, dev->irq, dev);
- pci_free_irq_vectors(pdev);
}
/* work with hotplug and coldplug */
--
2.25.0
[PATCH v4 4/4] i2c: thunderx: Use the correct name of device-managed function
Use the new function pcim_alloc_irq_vectors() to allocate IRQ vectors, the pcim_alloc_irq_vectors() function, an explicit device-managed version of pci_alloc_irq_vectors(). If pcim_enable_device() has been called before, then pci_alloc_irq_vectors() is actually a device-managed function. It is used here as a device-managed function, So replace it with pcim_alloc_irq_vectors(). Signed-off-by: Dejin Zheng --- v3 -> v4: - No change. v2 -> v3: - No change. v1 -> v2: - Modify some commit messages. drivers/i2c/busses/i2c-thunderx-pcidrv.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/i2c/busses/i2c-thunderx-pcidrv.c b/drivers/i2c/busses/i2c-thunderx-pcidrv.c index 12c90aa0900e..63354e9fb726 100644 --- a/drivers/i2c/busses/i2c-thunderx-pcidrv.c +++ b/drivers/i2c/busses/i2c-thunderx-pcidrv.c @@ -192,7 +192,7 @@ static int thunder_i2c_probe_pci(struct pci_dev *pdev, i2c->hlc_int_enable = thunder_i2c_hlc_int_enable; i2c->hlc_int_disable = thunder_i2c_hlc_int_disable; - ret = pci_alloc_irq_vectors(pdev, 1, 1, PCI_IRQ_MSIX); + ret = pcim_alloc_irq_vectors(pdev, 1, 1, PCI_IRQ_MSIX); if (ret < 0) goto error; -- 2.25.0
[PATCH v4 2/4] Documentation: devres: Add pcim_alloc_irq_vectors()
Add pcim_alloc_irq_vectors(), a device-managed version of pci_alloc_irq_vectors(). introducing this function can simplify the error handling path in many drivers. Signed-off-by: Dejin Zheng --- v3 -> v4: - No change v2 -> v3: - No change v1 -> v2: - Modify some commit messages. Documentation/driver-api/driver-model/devres.rst | 1 + 1 file changed, 1 insertion(+) diff --git a/Documentation/driver-api/driver-model/devres.rst b/Documentation/driver-api/driver-model/devres.rst index cd8b6e657b94..a52f65b6352f 100644 --- a/Documentation/driver-api/driver-model/devres.rst +++ b/Documentation/driver-api/driver-model/devres.rst @@ -380,6 +380,7 @@ PCI devm_pci_alloc_host_bridge() : managed PCI host bridge allocation devm_pci_remap_cfgspace(): ioremap PCI configuration space devm_pci_remap_cfg_resource(): ioremap PCI configuration space resource + pcim_alloc_irq_vectors() : managed IRQ vectors allocation pcim_enable_device() : after success, all PCI ops become managed pcim_pin_device(): keep PCI device enabled after release -- 2.25.0
[PATCH v4 1/4] PCI: Introduce pcim_alloc_irq_vectors()
Introduce pcim_alloc_irq_vectors(), a device-managed version of
pci_alloc_irq_vectors(). Introducing this function can simplify
the error handling path in many drivers.
And use pci_free_irq_vectors() to replace some code in pcim_release(),
they are equivalent, and no functional change. It is more explicit
that pcim_alloc_irq_vectors() is a device-managed function.
Suggested-by: Andy Shevchenko
Signed-off-by: Dejin Zheng
---
v3 -> v4:
- No change
v2 -> v3:
- Add some commit comments for replace some codes in
pcim_release() by pci_free_irq_vectors().
v1 -> v2:
- Use pci_free_irq_vectors() to replace some code in
pcim_release().
- Modify some commit messages.
drivers/pci/pci.c | 33 +
include/linux/pci.h | 3 +++
2 files changed, 32 insertions(+), 4 deletions(-)
diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index b67c4327d307..db799d089c85 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -1969,10 +1969,7 @@ static void pcim_release(struct device *gendev, void
*res)
struct pci_devres *this = res;
int i;
- if (dev->msi_enabled)
- pci_disable_msi(dev);
- if (dev->msix_enabled)
- pci_disable_msix(dev);
+ pci_free_irq_vectors(dev);
for (i = 0; i < DEVICE_COUNT_RESOURCE; i++)
if (this->region_mask & (1 << i))
@@ -2054,6 +2051,34 @@ void pcim_pin_device(struct pci_dev *pdev)
}
EXPORT_SYMBOL(pcim_pin_device);
+/**
+ * pcim_alloc_irq_vectors - a device-managed pci_alloc_irq_vectors()
+ * @dev: PCI device to operate on
+ * @min_vecs: minimum number of vectors required (must be >= 1)
+ * @max_vecs: maximum (desired) number of vectors
+ * @flags: flags or quirks for the allocation
+ *
+ * Return the number of vectors allocated, (which might be smaller than
+ * @max_vecs) if successful, or a negative error code on error. If less
+ * than @min_vecs interrupt vectors are available for @dev the function
+ * will fail with -ENOSPC.
+ *
+ * It depends on calling pcim_enable_device() to make IRQ resources
+ * manageable.
+ */
+int pcim_alloc_irq_vectors(struct pci_dev *dev, unsigned int min_vecs,
+ unsigned int max_vecs, unsigned int flags)
+{
+ struct pci_devres *dr;
+
+ dr = find_pci_dr(dev);
+ if (!dr || !dr->enabled)
+ return -EINVAL;
+
+ return pci_alloc_irq_vectors(dev, min_vecs, max_vecs, flags);
+}
+EXPORT_SYMBOL(pcim_alloc_irq_vectors);
+
/*
* pcibios_add_device - provide arch specific hooks when adding device dev
* @dev: the PCI device being added
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 86c799c97b77..d75ba85ddfc5 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -1818,6 +1818,9 @@ pci_alloc_irq_vectors(struct pci_dev *dev, unsigned int
min_vecs,
NULL);
}
+int pcim_alloc_irq_vectors(struct pci_dev *dev, unsigned int min_vecs,
+ unsigned int max_vecs, unsigned int flags);
+
/* Include architecture-dependent settings and functions */
#include
--
2.25.0
[PATCH v4 0/4] Introduce pcim_alloc_irq_vectors()
Introduce pcim_alloc_irq_vectors(), a device-managed version of pci_alloc_irq_vectors(), In some i2c drivers, If pcim_enable_device() has been called before, then pci_alloc_irq_vectors() is actually a device-managed function. It is used as a device-managed function, So replace it with pcim_alloc_irq_vectors(). Changelog - v3 -> v4: - add some commit comments for patch 3 v2 -> v3: - Add some commit comments for replace some codes in pcim_release() by pci_free_irq_vectors(). - Simplify the error handling path in i2c designware driver. v1 -> v2: - Use pci_free_irq_vectors() to replace some code in pcim_release(). - Modify some commit messages. Dejin Zheng (4): PCI: Introduce pcim_alloc_irq_vectors() Documentation: devres: Add pcim_alloc_irq_vectors() i2c: designware: Use the correct name of device-managed function i2c: thunderx: Use the correct name of device-managed function .../driver-api/driver-model/devres.rst| 1 + drivers/i2c/busses/i2c-designware-pcidrv.c| 15 +++-- drivers/i2c/busses/i2c-thunderx-pcidrv.c | 2 +- drivers/pci/pci.c | 33 --- include/linux/pci.h | 3 ++ 5 files changed, 38 insertions(+), 16 deletions(-) -- 2.25.0
Re: [PATCH v3 3/4] i2c: designware: Use the correct name of device-managed function
On Wed, Feb 17, 2021 at 03:47:01PM +0200, Andy Shevchenko wrote: > On Wed, Feb 17, 2021 at 07:40:14PM +0800, Dejin Zheng wrote: > > On Tue, Feb 16, 2021 at 06:46:01PM +0100, Krzysztof Wilczyński wrote: > > ... > > > > The change simplifies the error handling path, how? A line of two which > > > explains how it has been achieved might help should someone reads the > > > commit message in the future. > > > > > To put it simply, if the driver probe fail, the device-managed function > > mechanism will automatically call pcim_release(), then the > > pci_free_irq_vectors() > > will be executed. For details, please see the relevant code. > > Perhaps as a compromise you may add this short sentence to your commit > messages, like "the freeing resources will take automatically when device > is gone". > Ok, I will do it. Andy, Thanks for your help. And so sorry for the late reply. Yesterday was the last day of the Chinese New Year holiday. There were a lot of things to deal with. BR, Dejin > -- > With Best Regards, > Andy Shevchenko > >
Re: [PATCH v3 3/4] i2c: designware: Use the correct name of device-managed function
On Tue, Feb 16, 2021 at 06:46:01PM +0100, Krzysztof Wilczyński wrote:
Hi Krzysztof,
> Hi Dejin,
>
> Thank you for all the changes, looks good!
>
> You could improve the subject line, as it is very vague - what is the
> new function name more correct? Was the other and/or the previous one
> not correct? Seems like you are correcting a typo of sorts, rather than
> introducing a new function in this file.
>
If you have read the following commit comments, As you know, the
pci_alloc_irq_vectors() is not a real device-managed function. But
in some specific cases, it will act as an device-managed function.
Such naming will cause controversy, So In the case of need device-managed,
should be used pcim_alloc_irq_vectors(), an explicit device-managed
function. So the subject name is "Use the correct name of device-managed
function".
> > Use the new function pcim_alloc_irq_vectors() to allocate IRQ vectors,
> > the pcim_alloc_irq_vectors() function, an explicit device-managed
> > version of pci_alloc_irq_vectors(). If pcim_enable_device() has been
> > called before, then pci_alloc_irq_vectors() is actually
> > a device-managed function. It is used here as a device-managed
> > function, So replace it with pcim_alloc_irq_vectors().
>
> The commit is good, but it could use some polish, so to speak.
>
> A few suggestions to think about:
>
> - What are we adding and/or changing, and why
> - Why is using pcim_alloc_irq_vectors(), which is part
> of the managed devices framework, a better alternative
> to the pci_alloc_irq_vectors()
> - And finally why this change allowed us to remove the
> pci_free_irq_vectors()
>
These are all explained by the device-managed function mechanism.
> > At the same time, simplify the error handling path.
>
> The change simplifies the error handling path, how? A line of two which
> explains how it has been achieved might help should someone reads the
> commit message in the future.
>
To put it simply, if the driver probe fail, the device-managed function
mechanism will automatically call pcim_release(), then the
pci_free_irq_vectors()
will be executed. For details, please see the relevant code.
> [...]
> > if (controller->setup) {
> > r = controller->setup(pdev, controller);
> > - if (r) {
> > - pci_free_irq_vectors(pdev);
> > + if (r)
> > return r;
> > - }
> > }
> >
> > i2c_dw_adjust_bus_speed(dev);
> > @@ -246,10 +244,8 @@ static int i2c_dw_pci_probe(struct pci_dev *pdev,
> > i2c_dw_acpi_configure(>dev);
> >
> > r = i2c_dw_validate_speed(dev);
> > - if (r) {
> > - pci_free_irq_vectors(pdev);
> > + if (r)
> > return r;
> > - }
> >
> > i2c_dw_configure(dev);
> >
> > @@ -269,10 +265,8 @@ static int i2c_dw_pci_probe(struct pci_dev *pdev,
> > adap->nr = controller->bus_num;
> >
> > r = i2c_dw_probe(dev);
> > - if (r) {
> > - pci_free_irq_vectors(pdev);
> > + if (r)
> > return r;
> > - }
> >
> > pm_runtime_set_autosuspend_delay(>dev, 1000);
> > pm_runtime_use_autosuspend(>dev);
> > @@ -292,7 +286,6 @@ static void i2c_dw_pci_remove(struct pci_dev *pdev)
> >
> > i2c_del_adapter(>adapter);
> > devm_free_irq(>dev, dev->irq, dev);
> > - pci_free_irq_vectors(pdev);
>
> If pcim_release() is called should the pci_driver's probe callback fail,
Yes, you guessed right.
> and I assume that this is precisely the case, then all of the above make
> sense in the view of using pcim_alloc_irq_vectors().
>
> Reviewed-by: Krzysztof Wilczyński
>
> Krzysztof
BR,
Dejin
Re: [PATCH v3 2/4] Documentation: devres: Add pcim_alloc_irq_vectors()
On Tue, Feb 16, 2021 at 06:10:52PM +0100, Krzysztof Wilczyński wrote: > Hi Dejin, > > Thank you again for all the work here! > > > Add pcim_alloc_irq_vectors(), a device-managed version of > > pci_alloc_irq_vectors(). introducing this function can simplify > > the error handling path in many drivers. > > The second sentence should most likely start with a capital letter. > > Having said that, people might ask - how does it simplify the error > handling path? > > You might have to back this with a line of two to explain how does the > change achieved that, so that when someone looks at the commit message > it would be clear what the benefits of the change were. > Hi Krzysztof, The device-managed function is a conventional concept that every developer knows. So don't worry about this. And I really can't explain its operation mechanism to you in a sentence or two. If you are really interested, you can read the relevant code. I think it is meaningless to add a lot of explanations of general concepts in the commit comments. Maybe it will be better let us put more energy and time on the code. BR, Dejin > Reviewed-by: Krzysztof Wilczyński > > Krzysztof
Re: [PATCH v2 3/4] i2c: designware: Use the correct name of device-managed function
On Tue, Feb 16, 2021 at 04:39:09PM +0200, Andy Shevchenko wrote: > On Tue, Feb 16, 2021 at 10:18:09PM +0800, Dejin Zheng wrote: > > Use the new function pcim_alloc_irq_vectors() to allocate IRQ vectors, > > the pcim_alloc_irq_vectors() function, an explicit device-managed version > > of pci_alloc_irq_vectors(). If pcim_enable_device() has been called > > before, then pci_alloc_irq_vectors() is actually a device-managed > > function. It is used here as a device-managed function, So replace it > > with pcim_alloc_irq_vectors(). > > ... > > > - r = pci_alloc_irq_vectors(pdev, 1, 1, PCI_IRQ_ALL_TYPES); > > + r = pcim_alloc_irq_vectors(pdev, 1, 1, PCI_IRQ_ALL_TYPES); > > if (r < 0) > > return r; > > It's good, but now why do we have pci_free_irq_vectors() in the same file? > Done. and thank you for your careful inspection. > > -- > With Best Regards, > Andy Shevchenko > >
[PATCH v3 4/4] i2c: thunderx: Use the correct name of device-managed function
Use the new function pcim_alloc_irq_vectors() to allocate IRQ vectors, the pcim_alloc_irq_vectors() function, an explicit device-managed version of pci_alloc_irq_vectors(). If pcim_enable_device() has been called before, then pci_alloc_irq_vectors() is actually a device-managed function. It is used here as a device-managed function, So replace it with pcim_alloc_irq_vectors(). Signed-off-by: Dejin Zheng --- v2 -> v3: - No change. v1 -> v2: - Modify some commit messages. drivers/i2c/busses/i2c-thunderx-pcidrv.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/i2c/busses/i2c-thunderx-pcidrv.c b/drivers/i2c/busses/i2c-thunderx-pcidrv.c index 12c90aa0900e..63354e9fb726 100644 --- a/drivers/i2c/busses/i2c-thunderx-pcidrv.c +++ b/drivers/i2c/busses/i2c-thunderx-pcidrv.c @@ -192,7 +192,7 @@ static int thunder_i2c_probe_pci(struct pci_dev *pdev, i2c->hlc_int_enable = thunder_i2c_hlc_int_enable; i2c->hlc_int_disable = thunder_i2c_hlc_int_disable; - ret = pci_alloc_irq_vectors(pdev, 1, 1, PCI_IRQ_MSIX); + ret = pcim_alloc_irq_vectors(pdev, 1, 1, PCI_IRQ_MSIX); if (ret < 0) goto error; -- 2.25.0
[PATCH v3 2/4] Documentation: devres: Add pcim_alloc_irq_vectors()
Add pcim_alloc_irq_vectors(), a device-managed version of pci_alloc_irq_vectors(). introducing this function can simplify the error handling path in many drivers. Signed-off-by: Dejin Zheng --- v2 -> v3: - No change v1 -> v2: - Modify some commit messages. Documentation/driver-api/driver-model/devres.rst | 1 + 1 file changed, 1 insertion(+) diff --git a/Documentation/driver-api/driver-model/devres.rst b/Documentation/driver-api/driver-model/devres.rst index cd8b6e657b94..a52f65b6352f 100644 --- a/Documentation/driver-api/driver-model/devres.rst +++ b/Documentation/driver-api/driver-model/devres.rst @@ -380,6 +380,7 @@ PCI devm_pci_alloc_host_bridge() : managed PCI host bridge allocation devm_pci_remap_cfgspace(): ioremap PCI configuration space devm_pci_remap_cfg_resource(): ioremap PCI configuration space resource + pcim_alloc_irq_vectors() : managed IRQ vectors allocation pcim_enable_device() : after success, all PCI ops become managed pcim_pin_device(): keep PCI device enabled after release -- 2.25.0
[PATCH v3 3/4] i2c: designware: Use the correct name of device-managed function
Use the new function pcim_alloc_irq_vectors() to allocate IRQ vectors,
the pcim_alloc_irq_vectors() function, an explicit device-managed version
of pci_alloc_irq_vectors(). If pcim_enable_device() has been called
before, then pci_alloc_irq_vectors() is actually a device-managed
function. It is used here as a device-managed function, So replace it
with pcim_alloc_irq_vectors(). At the same time, simplify the error
handling path.
Signed-off-by: Dejin Zheng
---
v2 -> v3:
- simplify the error handling path.
v1 -> v2:
- Modify some commit messages.
drivers/i2c/busses/i2c-designware-pcidrv.c | 15 ---
1 file changed, 4 insertions(+), 11 deletions(-)
diff --git a/drivers/i2c/busses/i2c-designware-pcidrv.c
b/drivers/i2c/busses/i2c-designware-pcidrv.c
index 55c83a7a24f3..620b41e373b6 100644
--- a/drivers/i2c/busses/i2c-designware-pcidrv.c
+++ b/drivers/i2c/busses/i2c-designware-pcidrv.c
@@ -219,7 +219,7 @@ static int i2c_dw_pci_probe(struct pci_dev *pdev,
if (!dev)
return -ENOMEM;
- r = pci_alloc_irq_vectors(pdev, 1, 1, PCI_IRQ_ALL_TYPES);
+ r = pcim_alloc_irq_vectors(pdev, 1, 1, PCI_IRQ_ALL_TYPES);
if (r < 0)
return r;
@@ -234,10 +234,8 @@ static int i2c_dw_pci_probe(struct pci_dev *pdev,
if (controller->setup) {
r = controller->setup(pdev, controller);
- if (r) {
- pci_free_irq_vectors(pdev);
+ if (r)
return r;
- }
}
i2c_dw_adjust_bus_speed(dev);
@@ -246,10 +244,8 @@ static int i2c_dw_pci_probe(struct pci_dev *pdev,
i2c_dw_acpi_configure(>dev);
r = i2c_dw_validate_speed(dev);
- if (r) {
- pci_free_irq_vectors(pdev);
+ if (r)
return r;
- }
i2c_dw_configure(dev);
@@ -269,10 +265,8 @@ static int i2c_dw_pci_probe(struct pci_dev *pdev,
adap->nr = controller->bus_num;
r = i2c_dw_probe(dev);
- if (r) {
- pci_free_irq_vectors(pdev);
+ if (r)
return r;
- }
pm_runtime_set_autosuspend_delay(>dev, 1000);
pm_runtime_use_autosuspend(>dev);
@@ -292,7 +286,6 @@ static void i2c_dw_pci_remove(struct pci_dev *pdev)
i2c_del_adapter(>adapter);
devm_free_irq(>dev, dev->irq, dev);
- pci_free_irq_vectors(pdev);
}
/* work with hotplug and coldplug */
--
2.25.0
[PATCH v3 1/4] PCI: Introduce pcim_alloc_irq_vectors()
Introduce pcim_alloc_irq_vectors(), a device-managed version of
pci_alloc_irq_vectors(). Introducing this function can simplify
the error handling path in many drivers.
And use pci_free_irq_vectors() to replace some code in pcim_release(),
they are equivalent, and no functional change. It is more explicit
that pcim_alloc_irq_vectors() is a device-managed function.
Suggested-by: Andy Shevchenko
Signed-off-by: Dejin Zheng
---
v2 -> v3:
- Add some commit comments for replace some codes in
pcim_release() by pci_free_irq_vectors().
v1 -> v2:
- Use pci_free_irq_vectors() to replace some code in
pcim_release().
- Modify some commit messages.
drivers/pci/pci.c | 33 +
include/linux/pci.h | 3 +++
2 files changed, 32 insertions(+), 4 deletions(-)
diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index b67c4327d307..db799d089c85 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -1969,10 +1969,7 @@ static void pcim_release(struct device *gendev, void
*res)
struct pci_devres *this = res;
int i;
- if (dev->msi_enabled)
- pci_disable_msi(dev);
- if (dev->msix_enabled)
- pci_disable_msix(dev);
+ pci_free_irq_vectors(dev);
for (i = 0; i < DEVICE_COUNT_RESOURCE; i++)
if (this->region_mask & (1 << i))
@@ -2054,6 +2051,34 @@ void pcim_pin_device(struct pci_dev *pdev)
}
EXPORT_SYMBOL(pcim_pin_device);
+/**
+ * pcim_alloc_irq_vectors - a device-managed pci_alloc_irq_vectors()
+ * @dev: PCI device to operate on
+ * @min_vecs: minimum number of vectors required (must be >= 1)
+ * @max_vecs: maximum (desired) number of vectors
+ * @flags: flags or quirks for the allocation
+ *
+ * Return the number of vectors allocated, (which might be smaller than
+ * @max_vecs) if successful, or a negative error code on error. If less
+ * than @min_vecs interrupt vectors are available for @dev the function
+ * will fail with -ENOSPC.
+ *
+ * It depends on calling pcim_enable_device() to make IRQ resources
+ * manageable.
+ */
+int pcim_alloc_irq_vectors(struct pci_dev *dev, unsigned int min_vecs,
+ unsigned int max_vecs, unsigned int flags)
+{
+ struct pci_devres *dr;
+
+ dr = find_pci_dr(dev);
+ if (!dr || !dr->enabled)
+ return -EINVAL;
+
+ return pci_alloc_irq_vectors(dev, min_vecs, max_vecs, flags);
+}
+EXPORT_SYMBOL(pcim_alloc_irq_vectors);
+
/*
* pcibios_add_device - provide arch specific hooks when adding device dev
* @dev: the PCI device being added
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 86c799c97b77..d75ba85ddfc5 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -1818,6 +1818,9 @@ pci_alloc_irq_vectors(struct pci_dev *dev, unsigned int
min_vecs,
NULL);
}
+int pcim_alloc_irq_vectors(struct pci_dev *dev, unsigned int min_vecs,
+ unsigned int max_vecs, unsigned int flags);
+
/* Include architecture-dependent settings and functions */
#include
--
2.25.0
[PATCH v3 0/4] Introduce pcim_alloc_irq_vectors()
Introduce pcim_alloc_irq_vectors(), a device-managed version of pci_alloc_irq_vectors(), In some i2c drivers, If pcim_enable_device() has been called before, then pci_alloc_irq_vectors() is actually a device-managed function. It is used as a device-managed function, So replace it with pcim_alloc_irq_vectors(). Changelog - v2 -> v3: - Add some commit comments for replace some codes in pcim_release() by pci_free_irq_vectors(). - Simplify the error handling path in i2c designware driver. v1 -> v2: - Use pci_free_irq_vectors() to replace some code in pcim_release(). - Modify some commit messages. Dejin Zheng (4): PCI: Introduce pcim_alloc_irq_vectors() Documentation: devres: Add pcim_alloc_irq_vectors() i2c: designware: Use the correct name of device-managed function i2c: thunderx: Use the correct name of device-managed function .../driver-api/driver-model/devres.rst| 1 + drivers/i2c/busses/i2c-designware-pcidrv.c| 15 +++-- drivers/i2c/busses/i2c-thunderx-pcidrv.c | 2 +- drivers/pci/pci.c | 33 --- include/linux/pci.h | 3 ++ 5 files changed, 38 insertions(+), 16 deletions(-) -- 2.25.0
Re: [PATCH v1 1/4] PCI: Introduce pcim_alloc_irq_vectors()
On Tue, Feb 16, 2021 at 12:12:39PM +0200, Andy Shevchenko wrote: > On Mon, Feb 15, 2021 at 09:55:06PM +0100, Krzysztof Wilczyński wrote: > > > Question: wouldn't you need to call pci_free_irq_vectors() somewhere, > > possibly to pcim_release() callback? Although, I am not sure where the > > right place would be. > > > > I am asking, as the documentation (see [4]) suggests that one would have > > to release allocated IRQ vectors (relevant exceprt): > > It's done in pcim_release() but not explicitly. > > if (dev->msi_enabled) > pci_disable_msi(dev); > if (dev->msix_enabled) > pci_disable_msix(dev); > > Maybe above can be replaced by pci_free_irq_vectors() to be sure that any > future change to PCI IRQ allocation APIs. > > Yes, I have checked and currently the above code is equivalent to > pci_free_irq_vectors(). > > Dejin, please update your patch accordingly. > Hi Andy and Krzysztof, I have modified it and sent patch v2. thank you very much! BR, Dejin > > -- > With Best Regards, > Andy Shevchenko > >
[PATCH v2 4/4] i2c: thunderx: Use the correct name of device-managed function
Use the new function pcim_alloc_irq_vectors() to allocate IRQ vectors, the pcim_alloc_irq_vectors() function, an explicit device-managed version of pci_alloc_irq_vectors(). If pcim_enable_device() has been called before, then pci_alloc_irq_vectors() is actually a device-managed function. It is used here as a device-managed function, So replace it with pcim_alloc_irq_vectors(). Signed-off-by: Dejin Zheng --- v1 -> v2: - Modify some commit messages. drivers/i2c/busses/i2c-thunderx-pcidrv.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/i2c/busses/i2c-thunderx-pcidrv.c b/drivers/i2c/busses/i2c-thunderx-pcidrv.c index 12c90aa0900e..63354e9fb726 100644 --- a/drivers/i2c/busses/i2c-thunderx-pcidrv.c +++ b/drivers/i2c/busses/i2c-thunderx-pcidrv.c @@ -192,7 +192,7 @@ static int thunder_i2c_probe_pci(struct pci_dev *pdev, i2c->hlc_int_enable = thunder_i2c_hlc_int_enable; i2c->hlc_int_disable = thunder_i2c_hlc_int_disable; - ret = pci_alloc_irq_vectors(pdev, 1, 1, PCI_IRQ_MSIX); + ret = pcim_alloc_irq_vectors(pdev, 1, 1, PCI_IRQ_MSIX); if (ret < 0) goto error; -- 2.25.0
[PATCH v2 3/4] i2c: designware: Use the correct name of device-managed function
Use the new function pcim_alloc_irq_vectors() to allocate IRQ vectors, the pcim_alloc_irq_vectors() function, an explicit device-managed version of pci_alloc_irq_vectors(). If pcim_enable_device() has been called before, then pci_alloc_irq_vectors() is actually a device-managed function. It is used here as a device-managed function, So replace it with pcim_alloc_irq_vectors(). Signed-off-by: Dejin Zheng --- v1 -> v2: - Modify some commit messages. drivers/i2c/busses/i2c-designware-pcidrv.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/i2c/busses/i2c-designware-pcidrv.c b/drivers/i2c/busses/i2c-designware-pcidrv.c index 55c83a7a24f3..444533be49ee 100644 --- a/drivers/i2c/busses/i2c-designware-pcidrv.c +++ b/drivers/i2c/busses/i2c-designware-pcidrv.c @@ -219,7 +219,7 @@ static int i2c_dw_pci_probe(struct pci_dev *pdev, if (!dev) return -ENOMEM; - r = pci_alloc_irq_vectors(pdev, 1, 1, PCI_IRQ_ALL_TYPES); + r = pcim_alloc_irq_vectors(pdev, 1, 1, PCI_IRQ_ALL_TYPES); if (r < 0) return r; -- 2.25.0
[PATCH v2 1/4] PCI: Introduce pcim_alloc_irq_vectors()
Introduce pcim_alloc_irq_vectors(), a device-managed version of
pci_alloc_irq_vectors(). Introducing this function can simplify
the error handling path in many drivers.
Suggested-by: Andy Shevchenko
Signed-off-by: Dejin Zheng
---
v1 -> v2:
- Use pci_free_irq_vectors() to replace some code in
pcim_release().
- Modify some commit messages.
drivers/pci/pci.c | 33 +
include/linux/pci.h | 3 +++
2 files changed, 32 insertions(+), 4 deletions(-)
diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index b67c4327d307..db799d089c85 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -1969,10 +1969,7 @@ static void pcim_release(struct device *gendev, void
*res)
struct pci_devres *this = res;
int i;
- if (dev->msi_enabled)
- pci_disable_msi(dev);
- if (dev->msix_enabled)
- pci_disable_msix(dev);
+ pci_free_irq_vectors(dev);
for (i = 0; i < DEVICE_COUNT_RESOURCE; i++)
if (this->region_mask & (1 << i))
@@ -2054,6 +2051,34 @@ void pcim_pin_device(struct pci_dev *pdev)
}
EXPORT_SYMBOL(pcim_pin_device);
+/**
+ * pcim_alloc_irq_vectors - a device-managed pci_alloc_irq_vectors()
+ * @dev: PCI device to operate on
+ * @min_vecs: minimum number of vectors required (must be >= 1)
+ * @max_vecs: maximum (desired) number of vectors
+ * @flags: flags or quirks for the allocation
+ *
+ * Return the number of vectors allocated, (which might be smaller than
+ * @max_vecs) if successful, or a negative error code on error. If less
+ * than @min_vecs interrupt vectors are available for @dev the function
+ * will fail with -ENOSPC.
+ *
+ * It depends on calling pcim_enable_device() to make IRQ resources
+ * manageable.
+ */
+int pcim_alloc_irq_vectors(struct pci_dev *dev, unsigned int min_vecs,
+ unsigned int max_vecs, unsigned int flags)
+{
+ struct pci_devres *dr;
+
+ dr = find_pci_dr(dev);
+ if (!dr || !dr->enabled)
+ return -EINVAL;
+
+ return pci_alloc_irq_vectors(dev, min_vecs, max_vecs, flags);
+}
+EXPORT_SYMBOL(pcim_alloc_irq_vectors);
+
/*
* pcibios_add_device - provide arch specific hooks when adding device dev
* @dev: the PCI device being added
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 86c799c97b77..d75ba85ddfc5 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -1818,6 +1818,9 @@ pci_alloc_irq_vectors(struct pci_dev *dev, unsigned int
min_vecs,
NULL);
}
+int pcim_alloc_irq_vectors(struct pci_dev *dev, unsigned int min_vecs,
+ unsigned int max_vecs, unsigned int flags);
+
/* Include architecture-dependent settings and functions */
#include
--
2.25.0
[PATCH v2 0/4] Introduce pcim_alloc_irq_vectors()
Introduce pcim_alloc_irq_vectors(), a device-managed version of pci_alloc_irq_vectors(), In some i2c drivers, If pcim_enable_device() has been called before, then pci_alloc_irq_vectors() is actually a device-managed function. It is used as a device-managed function, So replace it with pcim_alloc_irq_vectors(). Changelog - v1 -> v2: - Use pci_free_irq_vectors() to replace some code in pcim_release(). - Modify some commit messages. Andy and Krzysztof, thanks for your help! Dejin Zheng (4): PCI: Introduce pcim_alloc_irq_vectors() Documentation: devres: Add pcim_alloc_irq_vectors() i2c: designware: Use the correct name of device-managed function i2c: thunderx: Use the correct name of device-managed function .../driver-api/driver-model/devres.rst| 1 + drivers/i2c/busses/i2c-designware-pcidrv.c| 2 +- drivers/i2c/busses/i2c-thunderx-pcidrv.c | 2 +- drivers/pci/pci.c | 33 --- include/linux/pci.h | 3 ++ 5 files changed, 35 insertions(+), 6 deletions(-) -- 2.25.0
[PATCH v2 2/4] Documentation: devres: Add pcim_alloc_irq_vectors()
Add pcim_alloc_irq_vectors(), a device-managed version of pci_alloc_irq_vectors(). introducing this function can simplify the error handling path in many drivers. Signed-off-by: Dejin Zheng --- v1 -> v2: - Modify some commit messages. Documentation/driver-api/driver-model/devres.rst | 1 + 1 file changed, 1 insertion(+) diff --git a/Documentation/driver-api/driver-model/devres.rst b/Documentation/driver-api/driver-model/devres.rst index cd8b6e657b94..a52f65b6352f 100644 --- a/Documentation/driver-api/driver-model/devres.rst +++ b/Documentation/driver-api/driver-model/devres.rst @@ -380,6 +380,7 @@ PCI devm_pci_alloc_host_bridge() : managed PCI host bridge allocation devm_pci_remap_cfgspace(): ioremap PCI configuration space devm_pci_remap_cfg_resource(): ioremap PCI configuration space resource + pcim_alloc_irq_vectors() : managed IRQ vectors allocation pcim_enable_device() : after success, all PCI ops become managed pcim_pin_device(): keep PCI device enabled after release -- 2.25.0
[PATCH v1 4/4] i2c: thunderx: Use the correct name of device-managed function
Use the correct name of device-managed function to alloc irq vectors, the pcim_alloc_irq_vectors() function, a explicit device-managed version of pci_alloc_irq_vectors(). Signed-off-by: Dejin Zheng --- drivers/i2c/busses/i2c-thunderx-pcidrv.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/i2c/busses/i2c-thunderx-pcidrv.c b/drivers/i2c/busses/i2c-thunderx-pcidrv.c index 12c90aa0900e..63354e9fb726 100644 --- a/drivers/i2c/busses/i2c-thunderx-pcidrv.c +++ b/drivers/i2c/busses/i2c-thunderx-pcidrv.c @@ -192,7 +192,7 @@ static int thunder_i2c_probe_pci(struct pci_dev *pdev, i2c->hlc_int_enable = thunder_i2c_hlc_int_enable; i2c->hlc_int_disable = thunder_i2c_hlc_int_disable; - ret = pci_alloc_irq_vectors(pdev, 1, 1, PCI_IRQ_MSIX); + ret = pcim_alloc_irq_vectors(pdev, 1, 1, PCI_IRQ_MSIX); if (ret < 0) goto error; -- 2.25.0
[PATCH v1 2/4] Documentation: devres: add pcim_alloc_irq_vectors()
add pcim_alloc_irq_vectors(), a explicit device-managed version of pci_alloc_irq_vectors(). Signed-off-by: Dejin Zheng --- Documentation/driver-api/driver-model/devres.rst | 1 + 1 file changed, 1 insertion(+) diff --git a/Documentation/driver-api/driver-model/devres.rst b/Documentation/driver-api/driver-model/devres.rst index cd8b6e657b94..cd53106fbf5e 100644 --- a/Documentation/driver-api/driver-model/devres.rst +++ b/Documentation/driver-api/driver-model/devres.rst @@ -380,6 +380,7 @@ PCI devm_pci_alloc_host_bridge() : managed PCI host bridge allocation devm_pci_remap_cfgspace(): ioremap PCI configuration space devm_pci_remap_cfg_resource(): ioremap PCI configuration space resource + pcim_alloc_irq_vectors() : managed irq vectors allocation pcim_enable_device() : after success, all PCI ops become managed pcim_pin_device(): keep PCI device enabled after release -- 2.25.0
[PATCH v1 3/4] i2c: designware: Use the correct name of device-managed function
Use the correct name of device-managed function to alloc irq vectors, the pcim_alloc_irq_vectors() function, a explicit device-managed version of pci_alloc_irq_vectors(). Signed-off-by: Dejin Zheng --- drivers/i2c/busses/i2c-designware-pcidrv.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/i2c/busses/i2c-designware-pcidrv.c b/drivers/i2c/busses/i2c-designware-pcidrv.c index 55c83a7a24f3..444533be49ee 100644 --- a/drivers/i2c/busses/i2c-designware-pcidrv.c +++ b/drivers/i2c/busses/i2c-designware-pcidrv.c @@ -219,7 +219,7 @@ static int i2c_dw_pci_probe(struct pci_dev *pdev, if (!dev) return -ENOMEM; - r = pci_alloc_irq_vectors(pdev, 1, 1, PCI_IRQ_ALL_TYPES); + r = pcim_alloc_irq_vectors(pdev, 1, 1, PCI_IRQ_ALL_TYPES); if (r < 0) return r; -- 2.25.0
[PATCH v1 0/4] Introduce pcim_alloc_irq_vectors()
Introduce pcim_alloc_irq_vectors(), a explicit device-managed version of pci_alloc_irq_vectors(). and use the correct name of device-managed function to alloc irq vectors in i2c drivers. Dejin Zheng (4): PCI: Introduce pcim_alloc_irq_vectors() Documentation: devres: add pcim_alloc_irq_vectors() i2c: designware: Use the correct name of device-managed function i2c: thunderx: Use the correct name of device-managed function .../driver-api/driver-model/devres.rst| 1 + drivers/i2c/busses/i2c-designware-pcidrv.c| 2 +- drivers/i2c/busses/i2c-thunderx-pcidrv.c | 2 +- drivers/pci/pci.c | 19 +++ include/linux/pci.h | 3 +++ 5 files changed, 25 insertions(+), 2 deletions(-) -- 2.25.0
[PATCH v1 1/4] PCI: Introduce pcim_alloc_irq_vectors()
Introduce pcim_alloc_irq_vectors(), a explicit device-managed version of
pci_alloc_irq_vectors().
Suggested-by: Andy Shevchenko
Signed-off-by: Dejin Zheng
---
drivers/pci/pci.c | 19 +++
include/linux/pci.h | 3 +++
2 files changed, 22 insertions(+)
diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index b67c4327d307..33244b512057 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -2054,6 +2054,25 @@ void pcim_pin_device(struct pci_dev *pdev)
}
EXPORT_SYMBOL(pcim_pin_device);
+/**
+ * pcim_alloc_irq_vectors - a device-managed pci_alloc_irq_vectors()
+ *
+ * It depends on calling pcim_enable_device() to make irq resources manageable.
+ */
+int pcim_alloc_irq_vectors(struct pci_dev *dev, unsigned int min_vecs,
+ unsigned int max_vecs, unsigned int flags)
+{
+ struct pci_devres *dr;
+
+ /*Ensure that the pcim_enable_device() function has been called*/
+ dr = find_pci_dr(dev);
+ if (!dr || !dr->enabled)
+ return -EINVAL;
+
+ return pci_alloc_irq_vectors(dev, min_vecs, max_vecs, flags);
+}
+EXPORT_SYMBOL(pcim_alloc_irq_vectors);
+
/*
* pcibios_add_device - provide arch specific hooks when adding device dev
* @dev: the PCI device being added
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 86c799c97b77..d75ba85ddfc5 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -1818,6 +1818,9 @@ pci_alloc_irq_vectors(struct pci_dev *dev, unsigned int
min_vecs,
NULL);
}
+int pcim_alloc_irq_vectors(struct pci_dev *dev, unsigned int min_vecs,
+ unsigned int max_vecs, unsigned int flags);
+
/* Include architecture-dependent settings and functions */
#include
--
2.25.0
Re: [PATCH] spi: pca2xx-pci: Fix an issue about missing call to 'pci_free_irq_vectors()'
On Mon, Feb 15, 2021 at 03:22:38PM +0200, Andy Shevchenko wrote:
> On Sun, Feb 14, 2021 at 10:57:46PM +0800, Dejin Zheng wrote:
> > Call to 'pci_free_irq_vectors()' are missing both in the error handling
> > path of the probe function, and in the remove function. So add them.
>
> I'm wondering if you noticed that it's done by pcim_* API.
> Perhaps you can introduce pcim_alloc_irq_vectors() or so and do not add these
> calls at all?
Andy, Thanks for your reminder, You mean introduce
pcim_alloc_irq_vectors() as like pcim_set_mwi() and free irq vectors by
pcim_release()?
>
> > Fixes: 64e02cb0bdfc7c ("spi: pca2xx-pci: Allow MSI")
>
> No, it doesn't fix anything.
>
> --
> With Best Regards,
> Andy Shevchenko
>
>
Re: [PATCH v2] spi: pxa2xx-pci: Fix an issue about missing call to 'pci_free_irq_vectors()'
On Mon, Feb 15, 2021 at 08:44:29PM +0800, Dejin Zheng wrote:
Hi All:
I'm very sorry forgot to add change list in patch v2.
v1 -> v2:
* modify subject name pca2xx-pci to pxa2xx-pci.
* add jan and Jarkko's review tag.
Dejin
> Call to 'pci_free_irq_vectors()' are missing both in the error handling
> path of the probe function, and in the remove function. So add them.
>
> Fixes: 64e02cb0bdfc7c ("spi: pca2xx-pci: Allow MSI")
> Reviewed-by: Jan Kiszka
> Reviewed-by: Jarkko Nikula
> Signed-off-by: Dejin Zheng
> ---
> drivers/spi/spi-pxa2xx-pci.c | 13 ++---
> 1 file changed, 10 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/spi/spi-pxa2xx-pci.c b/drivers/spi/spi-pxa2xx-pci.c
> index 14fc41ed2361..1ec840e78ff4 100644
> --- a/drivers/spi/spi-pxa2xx-pci.c
> +++ b/drivers/spi/spi-pxa2xx-pci.c
> @@ -254,8 +254,10 @@ static int pxa2xx_spi_pci_probe(struct pci_dev *dev,
> snprintf(buf, sizeof(buf), "pxa2xx-spi.%d", ssp->port_id);
> ssp->clk = clk_register_fixed_rate(>dev, buf , NULL, 0,
> c->max_clk_rate);
> - if (IS_ERR(ssp->clk))
> - return PTR_ERR(ssp->clk);
> + if (IS_ERR(ssp->clk)) {
> + ret = PTR_ERR(ssp->clk);
> + goto err_irq;
> + }
>
> memset(, 0, sizeof(pi));
> pi.fwnode = dev->dev.fwnode;
> @@ -268,12 +270,16 @@ static int pxa2xx_spi_pci_probe(struct pci_dev *dev,
> pdev = platform_device_register_full();
> if (IS_ERR(pdev)) {
> clk_unregister(ssp->clk);
> - return PTR_ERR(pdev);
> + ret = PTR_ERR(pdev);
> + goto err_irq;
> }
>
> pci_set_drvdata(dev, pdev);
>
> return 0;
> +err_irq:
> + pci_free_irq_vectors(dev);
> + return ret;
> }
>
> static void pxa2xx_spi_pci_remove(struct pci_dev *dev)
> @@ -283,6 +289,7 @@ static void pxa2xx_spi_pci_remove(struct pci_dev *dev)
>
> spi_pdata = dev_get_platdata(>dev);
>
> + pci_free_irq_vectors(dev);
> platform_device_unregister(pdev);
> clk_unregister(spi_pdata->ssp.clk);
> }
> --
> 2.25.0
>
Re: [PATCH 2/3] dmaengine: dw-edma: Add missing call to 'pci_free_irq_vectors()' in probe function
On Mon, Feb 15, 2021 at 09:45:07AM +, Gustavo Pimentel wrote:
> On Sun, Feb 14, 2021 at 13:21:52, Dejin Zheng
> wrote:
>
> > Call to 'pci_free_irq_vectors()' is missing in the error handling path
> > of the probe function, So add it.
> >
> > Fixes: 41aaff2a2ac01c5 ("dmaengine: Add Synopsys eDMA IP PCIe glue-logic")
> > Signed-off-by: Dejin Zheng
> > ---
> > drivers/dma/dw-edma/dw-edma-pcie.c | 15 +++
> > 1 file changed, 11 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/dma/dw-edma/dw-edma-pcie.c
> > b/drivers/dma/dw-edma/dw-edma-pcie.c
> > index 1eafc602e17e..c1e796bd3ee9 100644
> > --- a/drivers/dma/dw-edma/dw-edma-pcie.c
> > +++ b/drivers/dma/dw-edma/dw-edma-pcie.c
> > @@ -185,24 +185,31 @@ static int dw_edma_pcie_probe(struct pci_dev *pdev,
> > /* Validating if PCI interrupts were enabled */
> > if (!pci_dev_msi_enabled(pdev)) {
> > pci_err(pdev, "enable interrupt failed\n");
> > - return -EPERM;
> > + err = -EPERM;
> > + goto err_free_irq;
> > }
> >
> > dw->irq = devm_kcalloc(dev, nr_irqs, sizeof(*dw->irq), GFP_KERNEL);
> > - if (!dw->irq)
> > - return -ENOMEM;
> > + if (!dw->irq) {
> > + err = -ENOMEM;
> > + goto err_free_irq;
> > + }
> >
> > /* Starting eDMA driver */
> > err = dw_edma_probe(chip);
> > if (err) {
> > pci_err(pdev, "eDMA probe failed\n");
> > - return err;
> > + goto err_free_irq;
> > }
> >
> > /* Saving data structure reference */
> > pci_set_drvdata(pdev, chip);
> >
> > return 0;
> > +
> > +err_free_irq:
> > + pci_free_irq_vectors(pdev);
> > + return err;
> > }
> >
> > static void dw_edma_pcie_remove(struct pci_dev *pdev)
> > --
> > 2.25.0
>
> Acked-by: Gustavo Pimentel
>
>
Gustavo, Thanks!
Dejin
Re: [PATCH i2c-next] i2c: designware: Consolidate pci_free_irq_vectors to a single place
On Mon, Feb 15, 2021 at 11:36:27AM +0200, Jarkko Nikula wrote: > On 2/14/21 8:45 AM, Dejin Zheng wrote: > > Consolidate pci_free_irq_vectors to a single place using "goto free_irq" > > for simplify the code. > > > > Signed-off-by: Dejin Zheng > > --- > > drivers/i2c/busses/i2c-designware-pcidrv.c | 22 ++ > > 1 file changed, 10 insertions(+), 12 deletions(-) > > > Acked-by: Jarkko Nikula Jarkko, Thanks! Dejin
[PATCH v2] spi: pxa2xx-pci: Fix an issue about missing call to 'pci_free_irq_vectors()'
Call to 'pci_free_irq_vectors()' are missing both in the error handling
path of the probe function, and in the remove function. So add them.
Fixes: 64e02cb0bdfc7c ("spi: pca2xx-pci: Allow MSI")
Reviewed-by: Jan Kiszka
Reviewed-by: Jarkko Nikula
Signed-off-by: Dejin Zheng
---
drivers/spi/spi-pxa2xx-pci.c | 13 ++---
1 file changed, 10 insertions(+), 3 deletions(-)
diff --git a/drivers/spi/spi-pxa2xx-pci.c b/drivers/spi/spi-pxa2xx-pci.c
index 14fc41ed2361..1ec840e78ff4 100644
--- a/drivers/spi/spi-pxa2xx-pci.c
+++ b/drivers/spi/spi-pxa2xx-pci.c
@@ -254,8 +254,10 @@ static int pxa2xx_spi_pci_probe(struct pci_dev *dev,
snprintf(buf, sizeof(buf), "pxa2xx-spi.%d", ssp->port_id);
ssp->clk = clk_register_fixed_rate(>dev, buf , NULL, 0,
c->max_clk_rate);
-if (IS_ERR(ssp->clk))
- return PTR_ERR(ssp->clk);
+ if (IS_ERR(ssp->clk)) {
+ ret = PTR_ERR(ssp->clk);
+ goto err_irq;
+ }
memset(, 0, sizeof(pi));
pi.fwnode = dev->dev.fwnode;
@@ -268,12 +270,16 @@ static int pxa2xx_spi_pci_probe(struct pci_dev *dev,
pdev = platform_device_register_full();
if (IS_ERR(pdev)) {
clk_unregister(ssp->clk);
- return PTR_ERR(pdev);
+ ret = PTR_ERR(pdev);
+ goto err_irq;
}
pci_set_drvdata(dev, pdev);
return 0;
+err_irq:
+ pci_free_irq_vectors(dev);
+ return ret;
}
static void pxa2xx_spi_pci_remove(struct pci_dev *dev)
@@ -283,6 +289,7 @@ static void pxa2xx_spi_pci_remove(struct pci_dev *dev)
spi_pdata = dev_get_platdata(>dev);
+ pci_free_irq_vectors(dev);
platform_device_unregister(pdev);
clk_unregister(spi_pdata->ssp.clk);
}
--
2.25.0
Re: [PATCH] spi: pca2xx-pci: Fix an issue about missing call to 'pci_free_irq_vectors()'
On Mon, Feb 15, 2021 at 11:41:50AM +0200, Jarkko Nikula wrote:
> On 2/15/21 11:23 AM, Jan Kiszka wrote:
> > On 14.02.21 15:57, Dejin Zheng wrote:
> > > Call to 'pci_free_irq_vectors()' are missing both in the error handling
> > > path of the probe function, and in the remove function. So add them.
> > >
> > > Fixes: 64e02cb0bdfc7c ("spi: pca2xx-pci: Allow MSI")
> > > Signed-off-by: Dejin Zheng
> > > ---
> > > drivers/spi/spi-pxa2xx-pci.c | 13 ++---
> > > 1 file changed, 10 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/drivers/spi/spi-pxa2xx-pci.c b/drivers/spi/spi-pxa2xx-pci.c
> > > index 14fc41ed2361..1ec840e78ff4 100644
> > > --- a/drivers/spi/spi-pxa2xx-pci.c
> > > +++ b/drivers/spi/spi-pxa2xx-pci.c
> > > @@ -254,8 +254,10 @@ static int pxa2xx_spi_pci_probe(struct pci_dev *dev,
> > > snprintf(buf, sizeof(buf), "pxa2xx-spi.%d", ssp->port_id);
> > > ssp->clk = clk_register_fixed_rate(>dev, buf , NULL, 0,
> > > c->max_clk_rate);
> > > - if (IS_ERR(ssp->clk))
> > > - return PTR_ERR(ssp->clk);
> > > + if (IS_ERR(ssp->clk)) {
> > > + ret = PTR_ERR(ssp->clk);
> > > + goto err_irq;
> > > + }
> > > memset(, 0, sizeof(pi));
> > > pi.fwnode = dev->dev.fwnode;
> > > @@ -268,12 +270,16 @@ static int pxa2xx_spi_pci_probe(struct pci_dev *dev,
> > > pdev = platform_device_register_full();
> > > if (IS_ERR(pdev)) {
> > > clk_unregister(ssp->clk);
> > > - return PTR_ERR(pdev);
> > > + ret = PTR_ERR(pdev);
> > > + goto err_irq;
> > > }
> > > pci_set_drvdata(dev, pdev);
> > > return 0;
> > > +err_irq:
> > > + pci_free_irq_vectors(dev);
> > > + return ret;
> > > }
> > > static void pxa2xx_spi_pci_remove(struct pci_dev *dev)
> > > @@ -283,6 +289,7 @@ static void pxa2xx_spi_pci_remove(struct pci_dev *dev)
> > > spi_pdata = dev_get_platdata(>dev);
> > > + pci_free_irq_vectors(dev);
> > > platform_device_unregister(pdev);
> > > clk_unregister(spi_pdata->ssp.clk);
> > > }
> > >
> >
> > Reviewed-by: Jan Kiszka
> >
> Please fix pca2xx-pci -> pxa2xx-pci in the subject line. With that change
> you may add:
Jan and Jarkko, Thanks very much for your review. I will modify the
subject name in patch v2, Thanks again!
Dejin
>
> Reviewed-by: Jarkko Nikula
[PATCH] spi: pca2xx-pci: Fix an issue about missing call to 'pci_free_irq_vectors()'
Call to 'pci_free_irq_vectors()' are missing both in the error handling
path of the probe function, and in the remove function. So add them.
Fixes: 64e02cb0bdfc7c ("spi: pca2xx-pci: Allow MSI")
Signed-off-by: Dejin Zheng
---
drivers/spi/spi-pxa2xx-pci.c | 13 ++---
1 file changed, 10 insertions(+), 3 deletions(-)
diff --git a/drivers/spi/spi-pxa2xx-pci.c b/drivers/spi/spi-pxa2xx-pci.c
index 14fc41ed2361..1ec840e78ff4 100644
--- a/drivers/spi/spi-pxa2xx-pci.c
+++ b/drivers/spi/spi-pxa2xx-pci.c
@@ -254,8 +254,10 @@ static int pxa2xx_spi_pci_probe(struct pci_dev *dev,
snprintf(buf, sizeof(buf), "pxa2xx-spi.%d", ssp->port_id);
ssp->clk = clk_register_fixed_rate(>dev, buf , NULL, 0,
c->max_clk_rate);
-if (IS_ERR(ssp->clk))
- return PTR_ERR(ssp->clk);
+ if (IS_ERR(ssp->clk)) {
+ ret = PTR_ERR(ssp->clk);
+ goto err_irq;
+ }
memset(, 0, sizeof(pi));
pi.fwnode = dev->dev.fwnode;
@@ -268,12 +270,16 @@ static int pxa2xx_spi_pci_probe(struct pci_dev *dev,
pdev = platform_device_register_full();
if (IS_ERR(pdev)) {
clk_unregister(ssp->clk);
- return PTR_ERR(pdev);
+ ret = PTR_ERR(pdev);
+ goto err_irq;
}
pci_set_drvdata(dev, pdev);
return 0;
+err_irq:
+ pci_free_irq_vectors(dev);
+ return ret;
}
static void pxa2xx_spi_pci_remove(struct pci_dev *dev)
@@ -283,6 +289,7 @@ static void pxa2xx_spi_pci_remove(struct pci_dev *dev)
spi_pdata = dev_get_platdata(>dev);
+ pci_free_irq_vectors(dev);
platform_device_unregister(pdev);
clk_unregister(spi_pdata->ssp.clk);
}
--
2.25.0
[PATCH] i2c: thunderx: Fix an issue about missing call to 'pci_free_irq_vectors()'
Call to 'pci_free_irq_vectors()' are missing both in the error handling
path of the probe function, and in the remove function. So add them.
Fixes: 4c21541d8da17fb ("i2c: thunderx: Replace pci_enable_msix()")
Signed-off-by: Dejin Zheng
---
drivers/i2c/busses/i2c-thunderx-pcidrv.c | 9 ++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/drivers/i2c/busses/i2c-thunderx-pcidrv.c
b/drivers/i2c/busses/i2c-thunderx-pcidrv.c
index 12c90aa0900e..6a338be4d4b7 100644
--- a/drivers/i2c/busses/i2c-thunderx-pcidrv.c
+++ b/drivers/i2c/busses/i2c-thunderx-pcidrv.c
@@ -199,11 +199,11 @@ static int thunder_i2c_probe_pci(struct pci_dev *pdev,
ret = devm_request_irq(dev, pci_irq_vector(pdev, 0), octeon_i2c_isr, 0,
DRV_NAME, i2c);
if (ret)
- goto error;
+ goto error_free_irq;
ret = octeon_i2c_init_lowlevel(i2c);
if (ret)
- goto error;
+ goto error_free_irq;
octeon_i2c_set_clock(i2c);
@@ -219,7 +219,7 @@ static int thunder_i2c_probe_pci(struct pci_dev *pdev,
ret = i2c_add_adapter(>adap);
if (ret)
- goto error;
+ goto error_free_irq;
dev_info(i2c->dev, "Probed. Set system clock to %u\n", i2c->sys_freq);
@@ -229,6 +229,8 @@ static int thunder_i2c_probe_pci(struct pci_dev *pdev,
return 0;
+error_free_irq:
+ pci_free_irq_vectors(pdev);
error:
thunder_i2c_clock_disable(dev, i2c->clk);
return ret;
@@ -238,6 +240,7 @@ static void thunder_i2c_remove_pci(struct pci_dev *pdev)
{
struct octeon_i2c *i2c = pci_get_drvdata(pdev);
+ pci_free_irq_vectors(pdev);
thunder_i2c_smbus_remove(i2c);
thunder_i2c_clock_disable(>dev, i2c->clk);
i2c_del_adapter(>adap);
--
2.25.0
[PATCH 1/3] dmaengine: hsu: Add missing call to 'pci_free_irq_vectors()' in probe and remove functions
Call to 'pci_free_irq_vectors()' are missing both in the error handling
path of the probe function, and in the remove function.
Add them.
Fixes: e9bb8a9df316a2 ("dmaengine: hsu: pci: switch to new API for IRQ
allocation")
Signed-off-by: Dejin Zheng
---
drivers/dma/hsu/pci.c | 5 -
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/dma/hsu/pci.c b/drivers/dma/hsu/pci.c
index 9045a6f7f589..b335e2ef795b 100644
--- a/drivers/dma/hsu/pci.c
+++ b/drivers/dma/hsu/pci.c
@@ -89,7 +89,7 @@ static int hsu_pci_probe(struct pci_dev *pdev, const struct
pci_device_id *id)
ret = hsu_dma_probe(chip);
if (ret)
- return ret;
+ goto err_irq_vectors;
ret = request_irq(chip->irq, hsu_pci_irq, 0, "hsu_dma_pci", chip);
if (ret)
@@ -112,6 +112,8 @@ static int hsu_pci_probe(struct pci_dev *pdev, const struct
pci_device_id *id)
err_register_irq:
hsu_dma_remove(chip);
+err_irq_vectors:
+ pci_free_irq_vectors(pdev);
return ret;
}
@@ -121,6 +123,7 @@ static void hsu_pci_remove(struct pci_dev *pdev)
free_irq(chip->irq, chip);
hsu_dma_remove(chip);
+ pci_free_irq_vectors(pdev);
}
static const struct pci_device_id hsu_pci_id_table[] = {
--
2.25.0
[PATCH 3/3] dmaengine: hisilicon: Add missing call to 'pci_free_irq_vectors()' in probe function
Call to 'pci_free_irq_vectors()' is missing in the error handling path
of the probe function, So add it.
Fixes: e9f08b65250d73ab ("dmaengine: hisilicon: Add Kunpeng DMA engine support")
Signed-off-by: Dejin Zheng
---
drivers/dma/hisi_dma.c | 14 ++
1 file changed, 10 insertions(+), 4 deletions(-)
diff --git a/drivers/dma/hisi_dma.c b/drivers/dma/hisi_dma.c
index a259ee010e9b..9e894d7f5dab 100644
--- a/drivers/dma/hisi_dma.c
+++ b/drivers/dma/hisi_dma.c
@@ -553,7 +553,7 @@ static int hisi_dma_probe(struct pci_dev *pdev, const
struct pci_device_id *id)
ret = devm_add_action_or_reset(dev, hisi_dma_free_irq_vectors, pdev);
if (ret)
- return ret;
+ goto err_free_irq;
dma_dev = _dev->dma_dev;
dma_cap_set(DMA_MEMCPY, dma_dev->cap_mask);
@@ -572,18 +572,24 @@ static int hisi_dma_probe(struct pci_dev *pdev, const
struct pci_device_id *id)
ret = hisi_dma_enable_hw_channels(hdma_dev);
if (ret < 0) {
dev_err(dev, "failed to enable hw channel!\n");
- return ret;
+ goto err_free_irq;
}
ret = devm_add_action_or_reset(dev, hisi_dma_disable_hw_channels,
hdma_dev);
if (ret)
- return ret;
+ goto err_free_irq;
ret = dmaenginem_async_device_register(dma_dev);
- if (ret < 0)
+ if (ret < 0) {
dev_err(dev, "failed to register device!\n");
+ goto err_free_irq;
+ }
+
+ return ret;
+err_free_irq:
+ pci_free_irq_vectors(pdev);
return ret;
}
--
2.25.0
[PATCH 2/3] dmaengine: dw-edma: Add missing call to 'pci_free_irq_vectors()' in probe function
Call to 'pci_free_irq_vectors()' is missing in the error handling path
of the probe function, So add it.
Fixes: 41aaff2a2ac01c5 ("dmaengine: Add Synopsys eDMA IP PCIe glue-logic")
Signed-off-by: Dejin Zheng
---
drivers/dma/dw-edma/dw-edma-pcie.c | 15 +++
1 file changed, 11 insertions(+), 4 deletions(-)
diff --git a/drivers/dma/dw-edma/dw-edma-pcie.c
b/drivers/dma/dw-edma/dw-edma-pcie.c
index 1eafc602e17e..c1e796bd3ee9 100644
--- a/drivers/dma/dw-edma/dw-edma-pcie.c
+++ b/drivers/dma/dw-edma/dw-edma-pcie.c
@@ -185,24 +185,31 @@ static int dw_edma_pcie_probe(struct pci_dev *pdev,
/* Validating if PCI interrupts were enabled */
if (!pci_dev_msi_enabled(pdev)) {
pci_err(pdev, "enable interrupt failed\n");
- return -EPERM;
+ err = -EPERM;
+ goto err_free_irq;
}
dw->irq = devm_kcalloc(dev, nr_irqs, sizeof(*dw->irq), GFP_KERNEL);
- if (!dw->irq)
- return -ENOMEM;
+ if (!dw->irq) {
+ err = -ENOMEM;
+ goto err_free_irq;
+ }
/* Starting eDMA driver */
err = dw_edma_probe(chip);
if (err) {
pci_err(pdev, "eDMA probe failed\n");
- return err;
+ goto err_free_irq;
}
/* Saving data structure reference */
pci_set_drvdata(pdev, chip);
return 0;
+
+err_free_irq:
+ pci_free_irq_vectors(pdev);
+ return err;
}
static void dw_edma_pcie_remove(struct pci_dev *pdev)
--
2.25.0
[PATCH 0/3] Add missing call to 'pci_free_irq_vectors()'
This patchset just for add missing call to 'pci_free_irq_vectors()' in the error handling path of the probe function, or in the remove function. Dejin Zheng (3): dmaengine: hsu: Add missing call to 'pci_free_irq_vectors()' in probe and remove functions dmaengine: dw-edma: Add missing call to 'pci_free_irq_vectors()' in probe function dmaengine: hisilicon: Add missing call to 'pci_free_irq_vectors()' in probe function drivers/dma/dw-edma/dw-edma-pcie.c | 15 +++ drivers/dma/hisi_dma.c | 14 ++ drivers/dma/hsu/pci.c | 5 - 3 files changed, 25 insertions(+), 9 deletions(-) -- 2.25.0
[PATCH i2c-next] i2c: designware: Consolidate pci_free_irq_vectors to a single place
Consolidate pci_free_irq_vectors to a single place using "goto free_irq"
for simplify the code.
Signed-off-by: Dejin Zheng
---
drivers/i2c/busses/i2c-designware-pcidrv.c | 22 ++
1 file changed, 10 insertions(+), 12 deletions(-)
diff --git a/drivers/i2c/busses/i2c-designware-pcidrv.c
b/drivers/i2c/busses/i2c-designware-pcidrv.c
index 55c83a7a24f3..f0c82e91870a 100644
--- a/drivers/i2c/busses/i2c-designware-pcidrv.c
+++ b/drivers/i2c/busses/i2c-designware-pcidrv.c
@@ -234,10 +234,8 @@ static int i2c_dw_pci_probe(struct pci_dev *pdev,
if (controller->setup) {
r = controller->setup(pdev, controller);
- if (r) {
- pci_free_irq_vectors(pdev);
- return r;
- }
+ if (r)
+ goto free_irq;
}
i2c_dw_adjust_bus_speed(dev);
@@ -246,10 +244,8 @@ static int i2c_dw_pci_probe(struct pci_dev *pdev,
i2c_dw_acpi_configure(>dev);
r = i2c_dw_validate_speed(dev);
- if (r) {
- pci_free_irq_vectors(pdev);
- return r;
- }
+ if (r)
+ goto free_irq;
i2c_dw_configure(dev);
@@ -269,10 +265,8 @@ static int i2c_dw_pci_probe(struct pci_dev *pdev,
adap->nr = controller->bus_num;
r = i2c_dw_probe(dev);
- if (r) {
- pci_free_irq_vectors(pdev);
- return r;
- }
+ if (r)
+ goto free_irq;
pm_runtime_set_autosuspend_delay(>dev, 1000);
pm_runtime_use_autosuspend(>dev);
@@ -280,6 +274,10 @@ static int i2c_dw_pci_probe(struct pci_dev *pdev,
pm_runtime_allow(>dev);
return 0;
+
+free_irq:
+ pci_free_irq_vectors(pdev);
+ return r;
}
static void i2c_dw_pci_remove(struct pci_dev *pdev)
--
2.25.0
[PATCH mtd/next 1/8] mtd: Add helper macro for register_mtd_blktrans boilerplate
This patch introduces the module_mtd_blktrans macro which is a convenience macro for mtd blktrans modules similar to module_platform_driver. It is intended to be used by drivers which init/exit section does nothing but register/unregister the mtd blktrans driver. By using this macro it is possible to eliminate a few lines of boilerplate code per mtd blktrans driver. Signed-off-by: Dejin Zheng --- include/linux/mtd/blktrans.h | 11 +++ 1 file changed, 11 insertions(+) diff --git a/include/linux/mtd/blktrans.h b/include/linux/mtd/blktrans.h index 3c668cb1e344..15cc9b95e32b 100644 --- a/include/linux/mtd/blktrans.h +++ b/include/linux/mtd/blktrans.h @@ -77,5 +77,16 @@ extern int add_mtd_blktrans_dev(struct mtd_blktrans_dev *dev); extern int del_mtd_blktrans_dev(struct mtd_blktrans_dev *dev); extern int mtd_blktrans_cease_background(struct mtd_blktrans_dev *dev); +/** + * module_mtd_blktrans() - Helper macro for registering a mtd blktrans driver + * @__mtd_blktrans: mtd_blktrans_ops struct + * + * Helper macro for mtd blktrans drivers which do not do anything special in + * module init/exit. This eliminates a lot of boilerplate. Each module may only + * use this macro once, and calling it replaces module_init() and module_exit() + */ +#define module_mtd_blktrans(__mtd_blktrans) \ + module_driver(__mtd_blktrans, register_mtd_blktrans, \ + deregister_mtd_blktrans) #endif /* __MTD_TRANS_H__ */ -- 2.25.0
[PATCH mtd/next 5/8] mtd: mtdblock_ro: Use module_mtd_blktrans to register driver
Removing some boilerplate by using module_mtd_blktrans instead of calling
register and unregister in the otherwise empty init/exit functions.
Signed-off-by: Dejin Zheng
---
drivers/mtd/mtdblock_ro.c | 13 +
1 file changed, 1 insertion(+), 12 deletions(-)
diff --git a/drivers/mtd/mtdblock_ro.c b/drivers/mtd/mtdblock_ro.c
index 7fcf29ef2bdc..d92914f73d52 100644
--- a/drivers/mtd/mtdblock_ro.c
+++ b/drivers/mtd/mtdblock_ro.c
@@ -67,18 +67,7 @@ static struct mtd_blktrans_ops mtdblock_tr = {
.owner = THIS_MODULE,
};
-static int __init mtdblock_init(void)
-{
- return register_mtd_blktrans(_tr);
-}
-
-static void __exit mtdblock_exit(void)
-{
- deregister_mtd_blktrans(_tr);
-}
-
-module_init(mtdblock_init);
-module_exit(mtdblock_exit);
+module_mtd_blktrans(mtdblock_tr);
MODULE_LICENSE("GPL");
MODULE_AUTHOR("David Woodhouse ");
--
2.25.0
[PATCH mtd/next 7/8] mtd: nftlcore: Use module_mtd_blktrans to register driver
Removing some boilerplate by using module_mtd_blktrans instead of calling
register and unregister in the otherwise empty init/exit functions.
Signed-off-by: Dejin Zheng
---
drivers/mtd/nftlcore.c | 13 +
1 file changed, 1 insertion(+), 12 deletions(-)
diff --git a/drivers/mtd/nftlcore.c b/drivers/mtd/nftlcore.c
index d44641129cdb..bcd0094f172d 100644
--- a/drivers/mtd/nftlcore.c
+++ b/drivers/mtd/nftlcore.c
@@ -797,18 +797,7 @@ static struct mtd_blktrans_ops nftl_tr = {
.owner = THIS_MODULE,
};
-static int __init init_nftl(void)
-{
- return register_mtd_blktrans(_tr);
-}
-
-static void __exit cleanup_nftl(void)
-{
- deregister_mtd_blktrans(_tr);
-}
-
-module_init(init_nftl);
-module_exit(cleanup_nftl);
+module_mtd_blktrans(nftl_tr);
MODULE_LICENSE("GPL");
MODULE_AUTHOR("David Woodhouse , Fabrice Bellard
et al.");
--
2.25.0
[PATCH mtd/next 6/8] mtd: mtdswap: Use module_mtd_blktrans to register driver
Removing some boilerplate by using module_mtd_blktrans instead of calling
register and unregister in the otherwise empty init/exit functions.
Signed-off-by: Dejin Zheng
---
drivers/mtd/mtdswap.c | 14 +-
1 file changed, 1 insertion(+), 13 deletions(-)
diff --git a/drivers/mtd/mtdswap.c b/drivers/mtd/mtdswap.c
index 795dec4483c2..7e309270ddd4 100644
--- a/drivers/mtd/mtdswap.c
+++ b/drivers/mtd/mtdswap.c
@@ -1484,19 +1484,7 @@ static struct mtd_blktrans_ops mtdswap_ops = {
.owner = THIS_MODULE,
};
-static int __init mtdswap_modinit(void)
-{
- return register_mtd_blktrans(_ops);
-}
-
-static void __exit mtdswap_modexit(void)
-{
- deregister_mtd_blktrans(_ops);
-}
-
-module_init(mtdswap_modinit);
-module_exit(mtdswap_modexit);
-
+module_mtd_blktrans(mtdswap_ops);
MODULE_LICENSE("GPL");
MODULE_AUTHOR("Jarkko Lavinen ");
--
2.25.0
[PATCH mtd/next 8/8] mtd: rfd_ftl: Use module_mtd_blktrans to register driver
Removing some boilerplate by using module_mtd_blktrans instead of calling
register and unregister in the otherwise empty init/exit functions.
Signed-off-by: Dejin Zheng
---
drivers/mtd/rfd_ftl.c | 13 +
1 file changed, 1 insertion(+), 12 deletions(-)
diff --git a/drivers/mtd/rfd_ftl.c b/drivers/mtd/rfd_ftl.c
index 3d1df82fa105..cce3bf6f99b4 100644
--- a/drivers/mtd/rfd_ftl.c
+++ b/drivers/mtd/rfd_ftl.c
@@ -794,18 +794,7 @@ static struct mtd_blktrans_ops rfd_ftl_tr = {
.owner = THIS_MODULE,
};
-static int __init init_rfd_ftl(void)
-{
- return register_mtd_blktrans(_ftl_tr);
-}
-
-static void __exit cleanup_rfd_ftl(void)
-{
- deregister_mtd_blktrans(_ftl_tr);
-}
-
-module_init(init_rfd_ftl);
-module_exit(cleanup_rfd_ftl);
+module_mtd_blktrans(rfd_ftl_tr);
MODULE_LICENSE("GPL");
MODULE_AUTHOR("Sean Young ");
--
2.25.0
[PATCH mtd/next 3/8] mtd: inftlcore: Use module_mtd_blktrans to register driver
Removing some boilerplate by using module_mtd_blktrans instead of calling
register and unregister in the otherwise empty init/exit functions.
Signed-off-by: Dejin Zheng
---
drivers/mtd/inftlcore.c | 13 +
1 file changed, 1 insertion(+), 12 deletions(-)
diff --git a/drivers/mtd/inftlcore.c b/drivers/mtd/inftlcore.c
index a0d6c00e7b85..6b48397c750c 100644
--- a/drivers/mtd/inftlcore.c
+++ b/drivers/mtd/inftlcore.c
@@ -937,18 +937,7 @@ static struct mtd_blktrans_ops inftl_tr = {
.owner = THIS_MODULE,
};
-static int __init init_inftl(void)
-{
- return register_mtd_blktrans(_tr);
-}
-
-static void __exit cleanup_inftl(void)
-{
- deregister_mtd_blktrans(_tr);
-}
-
-module_init(init_inftl);
-module_exit(cleanup_inftl);
+module_mtd_blktrans(inftl_tr);
MODULE_LICENSE("GPL");
MODULE_AUTHOR("Greg Ungerer , David Woodhouse
, Fabrice Bellard et al.");
--
2.25.0
[PATCH mtd/next 4/8] mtd: mtdblock: Use module_mtd_blktrans to register driver
Removing some boilerplate by using module_mtd_blktrans instead of calling
register and unregister in the otherwise empty init/exit functions.
Signed-off-by: Dejin Zheng
---
drivers/mtd/mtdblock.c | 14 +-
1 file changed, 1 insertion(+), 13 deletions(-)
diff --git a/drivers/mtd/mtdblock.c b/drivers/mtd/mtdblock.c
index 32e52d83b961..a80809543793 100644
--- a/drivers/mtd/mtdblock.c
+++ b/drivers/mtd/mtdblock.c
@@ -346,19 +346,7 @@ static struct mtd_blktrans_ops mtdblock_tr = {
.owner = THIS_MODULE,
};
-static int __init init_mtdblock(void)
-{
- return register_mtd_blktrans(_tr);
-}
-
-static void __exit cleanup_mtdblock(void)
-{
- deregister_mtd_blktrans(_tr);
-}
-
-module_init(init_mtdblock);
-module_exit(cleanup_mtdblock);
-
+module_mtd_blktrans(mtdblock_tr);
MODULE_LICENSE("GPL");
MODULE_AUTHOR("Nicolas Pitre et al.");
--
2.25.0
[PATCH mtd/next 0/8] Introduces the module_mtd_blktrans macro
This patchset introduces the module_mtd_blktrans macro which is a convenience macro for mtd blktrans modules similar to module_platform_driver. It is intended to be used by drivers which init/exit section does nothing but register/unregister the mtd blktrans driver. By using this macro it is possible to eliminate a few lines of boilerplate code per mtd blktrans driver. Dejin Zheng (8): mtd: Add helper macro for register_mtd_blktrans boilerplate mtd: ftl: Use module_mtd_blktrans to register driver mtd: inftlcore: Use module_mtd_blktrans to register driver mtd: mtdblock: Use module_mtd_blktrans to register driver mtd: mtdblock_ro: Use module_mtd_blktrans to register driver mtd: mtdswap: Use module_mtd_blktrans to register driver mtd: nftlcore: Use module_mtd_blktrans to register driver mtd: rfd_ftl: Use module_mtd_blktrans to register driver drivers/mtd/ftl.c| 14 +- drivers/mtd/inftlcore.c | 13 + drivers/mtd/mtdblock.c | 14 +- drivers/mtd/mtdblock_ro.c| 13 + drivers/mtd/mtdswap.c| 14 +- drivers/mtd/nftlcore.c | 13 + drivers/mtd/rfd_ftl.c| 13 + include/linux/mtd/blktrans.h | 11 +++ 8 files changed, 18 insertions(+), 87 deletions(-) -- 2.25.0
[PATCH mtd/next 2/8] mtd: ftl: Use module_mtd_blktrans to register driver
Removing some boilerplate by using module_mtd_blktrans instead of calling
register and unregister in the otherwise empty init/exit functions.
Signed-off-by: Dejin Zheng
---
drivers/mtd/ftl.c | 14 +-
1 file changed, 1 insertion(+), 13 deletions(-)
diff --git a/drivers/mtd/ftl.c b/drivers/mtd/ftl.c
index 2578f27914ef..9b33c082179d 100644
--- a/drivers/mtd/ftl.c
+++ b/drivers/mtd/ftl.c
@@ -1056,19 +1056,7 @@ static struct mtd_blktrans_ops ftl_tr = {
.owner = THIS_MODULE,
};
-static int __init init_ftl(void)
-{
- return register_mtd_blktrans(_tr);
-}
-
-static void __exit cleanup_ftl(void)
-{
- deregister_mtd_blktrans(_tr);
-}
-
-module_init(init_ftl);
-module_exit(cleanup_ftl);
-
+module_mtd_blktrans(ftl_tr);
MODULE_LICENSE("Dual MPL/GPL");
MODULE_AUTHOR("David Hinds ");
--
2.25.0
Re: [PATCH v3] usb: dwc3: core: fix a issue about clear connect state
On Wed, Oct 28, 2020 at 03:57:03PM +0200, Felipe Balbi wrote: Hi Balbi and all: > > Hi, > > Dejin Zheng writes: > >> Dejin Zheng writes: > >> > According to Synopsys Programming Guide chapter 2.2 Register Resets, > >> > it cannot reset the DCTL register by setting DCTL.CSFTRST for core soft > >> > reset, if DWC3 controller as a slave device and stay connected with a usb > >> > host, then, while rebooting linux, it will fail to reinitialize dwc3 as a > >> > slave device when the DWC3 controller did not power off. because the > >> > connection status is incorrect, so we also need to clear DCTL.RUN_STOP > >> > bit for disabling connect when doing core soft reset. There will still > >> > be other stale configuration in DCTL, so reset the other fields of DCTL > >> > to the default value 0. > >> > >> This commit log is a bit hard to understand. When does this problem > >> actually happen? It seems like it's in the case of, perhaps, kexecing > >> into a new kernel, is that right? > >> > > It happens when entering the kernel for the second time after the reboot > > command. > > > >> At the time dwc3_core_soft_reset() is called, the assumption is that > >> we're starting with a clean core, from power up. If we have stale > >> configuration from a previous run, we should fix this on the exit > >> path. Note that if we're reaching probe with pull up connected, we > >> already have issues elsewhere. > >> > >> I think this is not the right fix for the problem. > >> > > I think you are right, Thinh also suggested me fix it on the exit path > > in the previous patch v2. Do you think I can do these cleanups in the > > shutdown hook of this driver? Balbi, is there a more suitable place to > > do this by your rich experience? Thanks! > > I don't think shutdown is called during removal, I'm not sure. I think > we had some fixes done in shutdown time, though. Test it out, but make > sure there are no issues with a regular modprobe cycle. > It has some errors in my commit message, I describe the process of linux restart is wrong. A PC is connected to our arm soc development board through the usb cable, the adb program runs via usb connection. there is a very important application in our linux system. when it goes wrong(halt or kernel panic), we want to restart linux. my wrong description happened here, when I manually kill this important application for testing, I thought it was calling the reboot command to restart linux, which is wrong. our real implementation is through watchdog, when the application no longer sets the watchdog and the watchdog times out, but watchdog can't reset the whole soc. our soc has 3 cpu clusters, one cluster has a arm Cortex R5 cpu for boot and security. one cluster has 2 arm Cortex A55 for linux system. the other cluster for android. when the Cortex R5 detect the watchdog timeout and want to restart linux system, it will stop Cortex A55 cpu to run, and load linux image to DDR memory from eMMC flash, then set Cortex A55 cpu to run new linux system, but it was not reset usb controller. so the usb controller's status is incorrect for boot new linux system. -- || |Boot and Security Linux Android| | | || 1 Cortex R5 || 2 Cortex A55 || 4 Cortex A72 | | ||cluster||cluster || cluster| | ||---||--||--| | || | SOC | |- Under normal circumstances, run the reboot command and rmmod the corresponding usb module, it will carry out the corresponding state processing, all of which can work well. Balbi, for this case, Currently, the way I can think of is to reset the DCTL register every initial time. Could you help me and give me some suggestions? thank you very much! BR, Dejin > -- > balbi
Re: [PATCH v3] usb: dwc3: core: fix a issue about clear connect state
On Tue, Oct 27, 2020 at 10:33:09AM +0200, Felipe Balbi wrote: Hi Balbi: Thank you so much for your comment. > > Hi, > > Dejin Zheng writes: > > According to Synopsys Programming Guide chapter 2.2 Register Resets, > > it cannot reset the DCTL register by setting DCTL.CSFTRST for core soft > > reset, if DWC3 controller as a slave device and stay connected with a usb > > host, then, while rebooting linux, it will fail to reinitialize dwc3 as a > > slave device when the DWC3 controller did not power off. because the > > connection status is incorrect, so we also need to clear DCTL.RUN_STOP > > bit for disabling connect when doing core soft reset. There will still > > be other stale configuration in DCTL, so reset the other fields of DCTL > > to the default value 0. > > This commit log is a bit hard to understand. When does this problem > actually happen? It seems like it's in the case of, perhaps, kexecing > into a new kernel, is that right? > It happens when entering the kernel for the second time after the reboot command. > At the time dwc3_core_soft_reset() is called, the assumption is that > we're starting with a clean core, from power up. If we have stale > configuration from a previous run, we should fix this on the exit > path. Note that if we're reaching probe with pull up connected, we > already have issues elsewhere. > > I think this is not the right fix for the problem. > I think you are right, Thinh also suggested me fix it on the exit path in the previous patch v2. Do you think I can do these cleanups in the shutdown hook of this driver? Balbi, is there a more suitable place to do this by your rich experience? Thanks! BR, Dejin > -- > balbi
Re: [PATCH v3] usb: dwc3: core: fix a issue about clear connect state
On Wed, Oct 28, 2020 at 03:57:03PM +0200, Felipe Balbi wrote: > > Hi, > > Dejin Zheng writes: > >> Dejin Zheng writes: > >> > According to Synopsys Programming Guide chapter 2.2 Register Resets, > >> > it cannot reset the DCTL register by setting DCTL.CSFTRST for core soft > >> > reset, if DWC3 controller as a slave device and stay connected with a usb > >> > host, then, while rebooting linux, it will fail to reinitialize dwc3 as a > >> > slave device when the DWC3 controller did not power off. because the > >> > connection status is incorrect, so we also need to clear DCTL.RUN_STOP > >> > bit for disabling connect when doing core soft reset. There will still > >> > be other stale configuration in DCTL, so reset the other fields of DCTL > >> > to the default value 0. > >> > >> This commit log is a bit hard to understand. When does this problem > >> actually happen? It seems like it's in the case of, perhaps, kexecing > >> into a new kernel, is that right? > >> > > It happens when entering the kernel for the second time after the reboot > > command. > > > >> At the time dwc3_core_soft_reset() is called, the assumption is that > >> we're starting with a clean core, from power up. If we have stale > >> configuration from a previous run, we should fix this on the exit > >> path. Note that if we're reaching probe with pull up connected, we > >> already have issues elsewhere. > >> > >> I think this is not the right fix for the problem. > >> > > I think you are right, Thinh also suggested me fix it on the exit path > > in the previous patch v2. Do you think I can do these cleanups in the > > shutdown hook of this driver? Balbi, is there a more suitable place to > > do this by your rich experience? Thanks! > > I don't think shutdown is called during removal, I'm not sure. I think > we had some fixes done in shutdown time, though. Test it out, but make > sure there are no issues with a regular modprobe cycle. > Balbi, thanks for your suggestions, I will do a test in the shutdown hook first. > -- > balbi
[PATCH v3] usb: dwc3: core: fix a issue about clear connect state
According to Synopsys Programming Guide chapter 2.2 Register Resets,
it cannot reset the DCTL register by setting DCTL.CSFTRST for core soft
reset, if DWC3 controller as a slave device and stay connected with a usb
host, then, while rebooting linux, it will fail to reinitialize dwc3 as a
slave device when the DWC3 controller did not power off. because the
connection status is incorrect, so we also need to clear DCTL.RUN_STOP
bit for disabling connect when doing core soft reset. There will still
be other stale configuration in DCTL, so reset the other fields of DCTL
to the default value 0.
Fixes: f59dcab176293b6 ("usb: dwc3: core: improve reset sequence")
Suggested-by: Thinh Nguyen
Signed-off-by: Dejin Zheng
---
v2 -> v3:
* Reset all fields of DCTL register by Thinh's suggest,
Thanks for Thinh's help!
v1 -> v2:
* modify some commit messages by Sergei's suggest, Thanks
very much for Sergei's help!
drivers/usb/dwc3/core.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
index 2eb34c8b4065..86375cfd9481 100644
--- a/drivers/usb/dwc3/core.c
+++ b/drivers/usb/dwc3/core.c
@@ -254,9 +254,7 @@ static int dwc3_core_soft_reset(struct dwc3 *dwc)
if (dwc->current_dr_role == DWC3_GCTL_PRTCAP_HOST)
return 0;
- reg = dwc3_readl(dwc->regs, DWC3_DCTL);
- reg |= DWC3_DCTL_CSFTRST;
- dwc3_writel(dwc->regs, DWC3_DCTL, reg);
+ dwc3_writel(dwc->regs, DWC3_DCTL, DWC3_DCTL_CSFTRST);
/*
* For DWC_usb31 controller 1.90a and later, the DCTL.CSFRST bit
--
2.25.0
Re: [PATCH v2] usb: dwc3: core: fix a issue about clear connect state
On Mon, Oct 19, 2020 at 10:04:41PM +, Thinh Nguyen wrote:
> Dejin Zheng wrote:
Hi Thinh:
> > According to Synopsys Programming Guide chapter 2.2 Register Resets,
> > it cannot reset the DCTL register by setting DCTL.CSFTRST for core soft
> > reset, if DWC3 controller as a slave device and stay connected with a usb
> > host, then, while rebooting linux, it will fail to reinitialize dwc3 as a
> > slave device when the DWC3 controller did not power off.
>
> If you reboot the OS, wouldn't it go through the driver tear-down
> sequence and clear the run_stop bit anyway?
Yes, you are right, this is a point worth checking. and I think it might
still be necessary to reset it.
> However, I can see how this can be an issue.
>
> > because the
> > connection status is incorrect, so we also need to clear DCTL.RUN_STOP
> > bit for disabling connect when doing core soft reset.
> >
> > Fixes: f59dcab176293b6 ("usb: dwc3: core: improve reset sequence")
> > Signed-off-by: Dejin Zheng
> > ---
> > v1 -> v2:
> > * modify some commit messages by Sergei's suggest, Thanks
> > very much for Sergei's help!
> >
> > drivers/usb/dwc3/core.c | 1 +
> > 1 file changed, 1 insertion(+)
> >
> > diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
> > index 2eb34c8b4065..239636c454c2 100644
> > --- a/drivers/usb/dwc3/core.c
> > +++ b/drivers/usb/dwc3/core.c
> > @@ -256,6 +256,7 @@ static int dwc3_core_soft_reset(struct dwc3 *dwc)
> >
> > reg = dwc3_readl(dwc->regs, DWC3_DCTL);
> > reg |= DWC3_DCTL_CSFTRST;
> > + reg &= ~DWC3_DCTL_RUN_STOP;
> > dwc3_writel(dwc->regs, DWC3_DCTL, reg);
> >
> > /*
>
> There will still be other stale configuration in DCTL if you do this. I
> think it's better to reset the other fields of DCTL to the default
> (should be all 0s) instead of doing register read-modify-write as what
> we're doing here. If not, at least we should use
> dwc3_gadget_dctl_write_safe().
>
Thinh, thanks very much for your suggestion, I think it might be better
to reset all areas of DCTL register. I tested it on my SOC platform and
it worked.
BR,
Dejin
> Thanks,
> Thinh
>
[PATCH v2] usb: dwc3: core: fix a issue about clear connect state
According to Synopsys Programming Guide chapter 2.2 Register Resets,
it cannot reset the DCTL register by setting DCTL.CSFTRST for core soft
reset, if DWC3 controller as a slave device and stay connected with a usb
host, then, while rebooting linux, it will fail to reinitialize dwc3 as a
slave device when the DWC3 controller did not power off. because the
connection status is incorrect, so we also need to clear DCTL.RUN_STOP
bit for disabling connect when doing core soft reset.
Fixes: f59dcab176293b6 ("usb: dwc3: core: improve reset sequence")
Signed-off-by: Dejin Zheng
---
v1 -> v2:
* modify some commit messages by Sergei's suggest, Thanks
very much for Sergei's help!
drivers/usb/dwc3/core.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
index 2eb34c8b4065..239636c454c2 100644
--- a/drivers/usb/dwc3/core.c
+++ b/drivers/usb/dwc3/core.c
@@ -256,6 +256,7 @@ static int dwc3_core_soft_reset(struct dwc3 *dwc)
reg = dwc3_readl(dwc->regs, DWC3_DCTL);
reg |= DWC3_DCTL_CSFTRST;
+ reg &= ~DWC3_DCTL_RUN_STOP;
dwc3_writel(dwc->regs, DWC3_DCTL, reg);
/*
--
2.25.0
[PATCH v1] usb: dwc3: core: fix a issue about clear connect state
According to Synopsys Programming Guide chapter 2.2 Register Resets,
it cannot reset the DCTL register by set DCTL.CSFTRST for Core Soft Reset,
if DWC3 controller as a slave device and stay connected with a usb host,
then, reboot linux, it will fail to reinitialize dwc3 as a slave device
when the DWC3 controller did not power off. because the connection status
is incorrect, so we also need clear DCTL.RUN_STOP bit for disable connect
when do core soft reset.
Fixes: f59dcab176293b6 ("usb: dwc3: core: improve reset sequence")
Signed-off-by: Dejin Zheng
---
drivers/usb/dwc3/core.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
index 2eb34c8b4065..239636c454c2 100644
--- a/drivers/usb/dwc3/core.c
+++ b/drivers/usb/dwc3/core.c
@@ -256,6 +256,7 @@ static int dwc3_core_soft_reset(struct dwc3 *dwc)
reg = dwc3_readl(dwc->regs, DWC3_DCTL);
reg |= DWC3_DCTL_CSFTRST;
+ reg &= ~DWC3_DCTL_RUN_STOP;
dwc3_writel(dwc->regs, DWC3_DCTL, reg);
/*
--
2.25.0
Re: [PATCH v4 2/2] i2c: busses: convert to devm_platform_request_irq()
On Fri, Jul 17, 2020 at 09:11:52PM +0200, Heiko Stübner wrote:
> Am Freitag, 17. Juli 2020, 18:11:58 CEST schrieb Dejin Zheng:
> > Use devm_platform_request_irq() to simplify code, and it contains
> > platform_get_irq() and devm_request_irq().
> >
> > Cc: Michal Simek
> > Cc: Wolfram Sang
> > Signed-off-by: Dejin Zheng
> > Acked-by: Linus Walleij
> > Acked-by: Michal Simek
>
>
> Rockchip part (i2c-rk3x):
> Reviewed-by: Heiko Stuebner
>
Heiko, Thanks a lot.
BR,
Dejin
> > diff --git a/drivers/i2c/busses/i2c-rk3x.c b/drivers/i2c/busses/i2c-rk3x.c
> > index 8e3cc85d1921..1f0ac69c5774 100644
> > --- a/drivers/i2c/busses/i2c-rk3x.c
> > +++ b/drivers/i2c/busses/i2c-rk3x.c
> > @@ -1227,7 +1227,6 @@ static int rk3x_i2c_probe(struct platform_device
> > *pdev)
> > int ret = 0;
> > int bus_nr;
> > u32 value;
> > - int irq;
> > unsigned long clk_rate;
> >
> > i2c = devm_kzalloc(>dev, sizeof(struct rk3x_i2c), GFP_KERNEL);
> > @@ -1289,17 +1288,10 @@ static int rk3x_i2c_probe(struct platform_device
> > *pdev)
> > }
> > }
> >
> > - /* IRQ setup */
> > - irq = platform_get_irq(pdev, 0);
> > - if (irq < 0)
> > - return irq;
> > -
> > - ret = devm_request_irq(>dev, irq, rk3x_i2c_irq,
> > - 0, dev_name(>dev), i2c);
> > - if (ret < 0) {
> > - dev_err(>dev, "cannot request IRQ\n");
> > + ret = devm_platform_request_irq(pdev, 0, NULL, rk3x_i2c_irq,
> > + 0, dev_name(>dev), i2c);
> > + if (ret < 0)
> > return ret;
> > - }
> >
> > platform_set_drvdata(pdev, i2c);
> >
>
>
>
>
Re: [PATCH v1] PCI: dwc: fix a warning about variable 'res' is uninitialized
On Fri, Jul 17, 2020 at 05:31:11PM +0100, Lorenzo Pieralisi wrote:
> On Fri, Jul 17, 2020 at 09:30:07PM +0800, Dejin Zheng wrote:
> > The kernel test robot reported a compile warning,
> >
> > drivers/pci/controller/dwc/pci-keystone.c:1236:18: warning: variable 'res'
> > is uninitialized when used here [-Wuninitialized]
> >
> > The commit c59a7d771134b5 ("PCI: dwc: Convert to
> > devm_platform_ioremap_resource_byname()") did a wrong conversion for
> > keystone driver. the commit use devm_platform_ioremap_resource_byname()
> > to replace platform_get_resource_byname() and devm_ioremap_resource().
> > but the subsequent code needs to use the variable 'res', which is got by
> > platform_get_resource_byname() for resource "app". so revert it.
> >
> > Fixes: c59a7d771134b5 ("PCI: dwc: Convert to
> > devm_platform_ioremap_resource_byname()")
> > Reported-by: kernel test robot
> > Signed-off-by: Dejin Zheng
> > ---
> > drivers/pci/controller/dwc/pci-keystone.c | 4 ++--
> > 1 file changed, 2 insertions(+), 2 deletions(-)
>
> Squashed in the commit it is fixing, thanks.
>
Lorenzo, Thanks a lot for your help.
BR,
Dejin
> Lorenzo
>
> > diff --git a/drivers/pci/controller/dwc/pci-keystone.c
> > b/drivers/pci/controller/dwc/pci-keystone.c
> > index 5ffc3b40c4f6..00279002102e 100644
> > --- a/drivers/pci/controller/dwc/pci-keystone.c
> > +++ b/drivers/pci/controller/dwc/pci-keystone.c
> > @@ -1228,8 +1228,8 @@ static int __init ks_pcie_probe(struct
> > platform_device *pdev)
> > if (!pci)
> > return -ENOMEM;
> >
> > - ks_pcie->va_app_base =
> > - devm_platform_ioremap_resource_byname(pdev, "app");
> > + res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "app");
> > + ks_pcie->va_app_base = devm_ioremap_resource(dev, res);
> > if (IS_ERR(ks_pcie->va_app_base))
> > return PTR_ERR(ks_pcie->va_app_base);
> >
> > --
> > 2.25.0
> >
[PATCH v4 2/2] i2c: busses: convert to devm_platform_request_irq()
Use devm_platform_request_irq() to simplify code, and it contains
platform_get_irq() and devm_request_irq().
Cc: Michal Simek
Cc: Wolfram Sang
Signed-off-by: Dejin Zheng
Acked-by: Linus Walleij
Acked-by: Michal Simek
---
v3 -> v4:
- add Michal's Acked-by tag in the second patch. and Thanks
for Michal's help.
v2 -> v3:
- add Linus's Acked-by tag and Thanks linus's review.
v1 -> v2:
- The patch content has not changed. just resend it by this discussion:
https://patchwork.ozlabs.org/project/linux-i2c/patch/20200520144821.8069-1-zhengdej...@gmail.com/
drivers/i2c/busses/i2c-bcm-kona.c | 16 +++-
drivers/i2c/busses/i2c-cadence.c | 10 +++---
drivers/i2c/busses/i2c-digicolor.c | 10 +++---
drivers/i2c/busses/i2c-emev2.c | 5 ++---
drivers/i2c/busses/i2c-jz4780.c| 5 ++---
drivers/i2c/busses/i2c-meson.c | 13 -
drivers/i2c/busses/i2c-mxs.c | 9 +++--
drivers/i2c/busses/i2c-pnx.c | 9 ++---
drivers/i2c/busses/i2c-rcar.c | 9 +++--
drivers/i2c/busses/i2c-rk3x.c | 14 +++---
drivers/i2c/busses/i2c-sirf.c | 10 ++
drivers/i2c/busses/i2c-stu300.c| 4 ++--
drivers/i2c/busses/i2c-synquacer.c | 12 +++-
13 files changed, 35 insertions(+), 91 deletions(-)
diff --git a/drivers/i2c/busses/i2c-bcm-kona.c
b/drivers/i2c/busses/i2c-bcm-kona.c
index ed5e1275ae46..f45acb47552a 100644
--- a/drivers/i2c/busses/i2c-bcm-kona.c
+++ b/drivers/i2c/busses/i2c-bcm-kona.c
@@ -818,20 +818,10 @@ static int bcm_kona_i2c_probe(struct platform_device
*pdev)
ISR_NOACK_MASK,
dev->base + ISR_OFFSET);
- /* Get the interrupt number */
- dev->irq = platform_get_irq(pdev, 0);
- if (dev->irq < 0) {
- rc = dev->irq;
- goto probe_disable_clk;
- }
-
- /* register the ISR handler */
- rc = devm_request_irq(>dev, dev->irq, bcm_kona_i2c_isr,
- IRQF_SHARED, pdev->name, dev);
- if (rc) {
- dev_err(dev->device, "failed to request irq %i\n", dev->irq);
+ rc = devm_platform_request_irq(pdev, 0, >irq, bcm_kona_i2c_isr,
+ IRQF_SHARED, pdev->name, dev);
+ if (rc)
goto probe_disable_clk;
- }
/* Enable the controller but leave it idle */
bcm_kona_i2c_send_cmd_to_ctrl(dev, BCM_CMD_NOACTION);
diff --git a/drivers/i2c/busses/i2c-cadence.c b/drivers/i2c/busses/i2c-cadence.c
index 4b72398af505..9ffae4d231dc 100644
--- a/drivers/i2c/busses/i2c-cadence.c
+++ b/drivers/i2c/busses/i2c-cadence.c
@@ -1204,8 +1204,6 @@ static int cdns_i2c_probe(struct platform_device *pdev)
if (IS_ERR(id->membase))
return PTR_ERR(id->membase);
- id->irq = platform_get_irq(pdev, 0);
-
id->adap.owner = THIS_MODULE;
id->adap.dev.of_node = pdev->dev.of_node;
id->adap.algo = _i2c_algo;
@@ -1256,12 +1254,10 @@ static int cdns_i2c_probe(struct platform_device *pdev)
goto err_clk_dis;
}
- ret = devm_request_irq(>dev, id->irq, cdns_i2c_isr, 0,
-DRIVER_NAME, id);
- if (ret) {
- dev_err(>dev, "cannot get irq %d\n", id->irq);
+ ret = devm_platform_request_irq(pdev, 0, >irq, cdns_i2c_isr, 0,
+ DRIVER_NAME, id);
+ if (ret)
goto err_clk_dis;
- }
/*
* Cadence I2C controller has a bug wherein it generates
diff --git a/drivers/i2c/busses/i2c-digicolor.c
b/drivers/i2c/busses/i2c-digicolor.c
index 332f00437479..9ea965f41396 100644
--- a/drivers/i2c/busses/i2c-digicolor.c
+++ b/drivers/i2c/busses/i2c-digicolor.c
@@ -290,7 +290,7 @@ static int dc_i2c_probe(struct platform_device *pdev)
{
struct device_node *np = pdev->dev.of_node;
struct dc_i2c *i2c;
- int ret = 0, irq;
+ int ret = 0;
i2c = devm_kzalloc(>dev, sizeof(struct dc_i2c), GFP_KERNEL);
if (!i2c)
@@ -314,12 +314,8 @@ static int dc_i2c_probe(struct platform_device *pdev)
if (IS_ERR(i2c->regs))
return PTR_ERR(i2c->regs);
- irq = platform_get_irq(pdev, 0);
- if (irq < 0)
- return irq;
-
- ret = devm_request_irq(>dev, irq, dc_i2c_irq, 0,
- dev_name(>dev), i2c);
+ ret = devm_platform_request_irq(pdev, 0, NULL, dc_i2c_irq, 0,
+ dev_name(>dev), i2c);
if (ret < 0)
return ret;
diff --git a/drivers/i2c/busses/i2c-emev2.c b/drivers/i2c/busses/i2c-emev2.c
index a08554c1a570..1a605d7b56dc 100644
--- a/drivers/i2c/busses/i2c-emev2.c
+++ b/drivers/i2c/busses/i2c-emev2.c
@@ -395,9 +395,8 @@ static int em_i2c
[PATCH v4 1/2] drivers: provide devm_platform_request_irq()
It will call devm_request_irq() after platform_get_irq() function
in many drivers, And sometimes, the error handling of these two functions
is incorrect in some drivers. So this function is provided to simplify
the driver.
Cc: Michal Simek
Cc: Wolfram Sang
Signed-off-by: Dejin Zheng
---
v3 -> v4:
- The patch v3 sent on May 27 may be lost somewhere in the
world, so resend it.
v2 -> v3:
- add devm_platform_request_irq() to devres.rst by Grygorii's
suggestion.
v1 -> v2:
- The patch content has not changed. just resend it by this discussion:
https://patchwork.ozlabs.org/project/linux-i2c/patch/20200520144821.8069-1-zhengdej...@gmail.com/
.../driver-api/driver-model/devres.rst| 1 +
drivers/base/platform.c | 33 +++
include/linux/platform_device.h | 4 +++
3 files changed, 38 insertions(+)
diff --git a/Documentation/driver-api/driver-model/devres.rst
b/Documentation/driver-api/driver-model/devres.rst
index efc21134..9c75903bce7c 100644
--- a/Documentation/driver-api/driver-model/devres.rst
+++ b/Documentation/driver-api/driver-model/devres.rst
@@ -326,6 +326,7 @@ IRQ
devm_request_any_context_irq()
devm_request_irq()
devm_request_threaded_irq()
+ devm_platform_request_irq()
devm_irq_alloc_descs()
devm_irq_alloc_desc()
devm_irq_alloc_desc_at()
diff --git a/drivers/base/platform.c b/drivers/base/platform.c
index e5d8a0503b4f..d4212d12bf60 100644
--- a/drivers/base/platform.c
+++ b/drivers/base/platform.c
@@ -275,6 +275,39 @@ int platform_irq_count(struct platform_device *dev)
}
EXPORT_SYMBOL_GPL(platform_irq_count);
+/**
+ * devm_platform_request_irq - get an irq and allocate an interrupt
+ * line for a managed device
+ * @pdev: platform device
+ * @num: IRQ number index
+ * @irq: get an IRQ for a device if irq != NULL
+ * @handler: function to be called when the IRQ occurs
+ * @irqflags: interrupt type flags
+ * @devname: an ascii name for the claiming device, dev_name(dev) if NULL
+ * @dev_id: a cookie passed back to the handler function
+ *
+ * Return: zero on success, negative error number on failure.
+ */
+int devm_platform_request_irq(struct platform_device *pdev, unsigned int num,
+ unsigned int *irq, irq_handler_t handler,
+ unsigned long irqflags, const char *devname, void *dev_id)
+{
+ int tmp_irq, ret;
+
+ tmp_irq = platform_get_irq(pdev, num);
+ if (tmp_irq < 0)
+ return tmp_irq;
+
+ ret = devm_request_irq(>dev, tmp_irq, handler, irqflags,
+ devname, dev_id);
+ if (ret < 0)
+ dev_err(>dev, "can't request IRQ\n");
+ else if (irq != NULL)
+ *irq = tmp_irq;
+ return ret;
+}
+EXPORT_SYMBOL_GPL(devm_platform_request_irq);
+
/**
* platform_get_resource_byname - get a resource for a device by name
* @dev: platform device
diff --git a/include/linux/platform_device.h b/include/linux/platform_device.h
index 77a2aada106d..d94652deea5c 100644
--- a/include/linux/platform_device.h
+++ b/include/linux/platform_device.h
@@ -11,6 +11,7 @@
#define _PLATFORM_DEVICE_H_
#include
+#include
#define PLATFORM_DEVID_NONE(-1)
#define PLATFORM_DEVID_AUTO(-2)
@@ -70,6 +71,9 @@ devm_platform_ioremap_resource_byname(struct platform_device
*pdev,
extern int platform_get_irq(struct platform_device *, unsigned int);
extern int platform_get_irq_optional(struct platform_device *, unsigned int);
extern int platform_irq_count(struct platform_device *);
+extern int devm_platform_request_irq(struct platform_device *pdev,
+ unsigned int num, unsigned int *irq, irq_handler_t handler,
+ unsigned long irqflags, const char *devname, void *dev_id);
extern struct resource *platform_get_resource_byname(struct platform_device *,
unsigned int,
const char *);
--
2.25.0
[PATCH v4 0/2] drivers: provide devm_platform_request_irq()
It will call devm_request_irq() after platform_get_irq() function in many drivers, And sometimes, the error handling of these two functions is incorrect in some drivers. So this function is provided to simplify the driver. the first patch will provide devm_platform_request_irq(), and the second patch will convert to devm_platform_request_irq() in some dirver of i2c bus. v3 -> v4: - The patch v3 sent on May 27 may be lost somewhere in the world, so resend it. - add Michal's Acked-by tag in the second patch. and Thanks for Michal's help. v2 -> v3: - add devm_platform_request_irq() to devres.rst by Grygorii's suggestion. - And also Thanks Michal, Wolfram and Linus's review and comments. v1 -> v2: - I give up this series of patches in v1 version. I resend this patches v2 by that discussion: https://patchwork.ozlabs.org/project/linux-i2c/patch/20200520144821.8069-1-zhengdej...@gmail.com/ The patch content has not changed. Dejin Zheng (2): drivers: provide devm_platform_request_irq() i2c: busses: convert to devm_platform_request_irq() .../driver-api/driver-model/devres.rst| 1 + drivers/base/platform.c | 33 +++ drivers/i2c/busses/i2c-bcm-kona.c | 16 ++--- drivers/i2c/busses/i2c-cadence.c | 10 ++ drivers/i2c/busses/i2c-digicolor.c| 10 ++ drivers/i2c/busses/i2c-emev2.c| 5 ++- drivers/i2c/busses/i2c-jz4780.c | 5 ++- drivers/i2c/busses/i2c-meson.c| 13 +++- drivers/i2c/busses/i2c-mxs.c | 9 ++--- drivers/i2c/busses/i2c-pnx.c | 9 ++--- drivers/i2c/busses/i2c-rcar.c | 9 ++--- drivers/i2c/busses/i2c-rk3x.c | 14 ++-- drivers/i2c/busses/i2c-sirf.c | 10 ++ drivers/i2c/busses/i2c-stu300.c | 4 +-- drivers/i2c/busses/i2c-synquacer.c| 12 ++- include/linux/platform_device.h | 4 +++ 16 files changed, 73 insertions(+), 91 deletions(-) -- 2.25.0
Re: [PATCH v3 1/2] drivers: provide devm_platform_request_irq()
On Fri, Jul 17, 2020 at 04:59:46PM +0200, Greg KH wrote: > On Fri, Jul 17, 2020 at 10:30:30PM +0800, Dejin Zheng wrote: > > On Wed, May 27, 2020 at 10:26:10PM +0800, Dejin Zheng wrote: > > Hi Jonathan, Greg, Rafael, Linus, Michal and Wolfram: > > > > Could you help me review this patch if you have free time? > > It's not anywhere in queue, sorry, please resend it? > Ok, I will resend it. Thanks very much! BR, Dejin > thanks, > > greg k-h
Re: [PATCH v3 1/2] drivers: provide devm_platform_request_irq()
On Wed, May 27, 2020 at 10:26:10PM +0800, Dejin Zheng wrote: Hi Jonathan, Greg, Rafael, Linus, Michal and Wolfram: Could you help me review this patch if you have free time? Thank you very very much! BR, Dejin > It will call devm_request_irq() after platform_get_irq() function > in many drivers, sometimes, it is not right for the error handling > of these two functions in some drivers. so provide this function > to simplify the driver. > > Cc: Michal Simek > Cc: Wolfram Sang > Signed-off-by: Dejin Zheng > --- > v2 -> v3: > - add devm_platform_request_irq() to devres.rst by Grygorii's > suggestion. > v1 -> v2: > - The patch content has not changed. just resend it by this discussion: > > https://patchwork.ozlabs.org/project/linux-i2c/patch/20200520144821.8069-1-zhengdej...@gmail.com/ >
Re: [PATCH v1] PCI: controller: Remove duplicate error message
On Wed, Jul 15, 2020 at 05:45:59PM +0100, Lorenzo Pieralisi wrote: > On Sat, Jul 11, 2020 at 03:47:33PM +0800, Dejin Zheng wrote: > > On Mon, Jul 06, 2020 at 04:58:47PM +0100, Lorenzo Pieralisi wrote: > > > On Tue, Jun 02, 2020 at 09:01:13AM -0600, Rob Herring wrote: > > > > > > [...] > > > > > > > > The other 2 error cases as well don't print the resource name as far > > > > > as > > > > > I recall (they will at least print the resource start/end). > > > > > > > > Start/end are what are important for why either of these functions > > > > failed. > > > > > > > > But sure, we could add 'name' here. That's a separate patch IMO. > > > > Hi Lorenzo, Bob and Jonathan: > > > > > > Thank you very much for helping me review this patch, > > I merge this patch in pci/misc, thanks. > > > I sent a new patch for print the resource name when the request memory > > region or remapping of configuration space fails. and it is here: > > https://patchwork.kernel.org/patch/11657801/ > > We will get to it soon. > > Lorenzo Lorenzo, Thanks very much! Dejin
[PATCH v1] PCI: dwc: fix a warning about variable 'res' is uninitialized
The kernel test robot reported a compile warning,
drivers/pci/controller/dwc/pci-keystone.c:1236:18: warning: variable 'res'
is uninitialized when used here [-Wuninitialized]
The commit c59a7d771134b5 ("PCI: dwc: Convert to
devm_platform_ioremap_resource_byname()") did a wrong conversion for
keystone driver. the commit use devm_platform_ioremap_resource_byname()
to replace platform_get_resource_byname() and devm_ioremap_resource().
but the subsequent code needs to use the variable 'res', which is got by
platform_get_resource_byname() for resource "app". so revert it.
Fixes: c59a7d771134b5 ("PCI: dwc: Convert to
devm_platform_ioremap_resource_byname()")
Reported-by: kernel test robot
Signed-off-by: Dejin Zheng
---
drivers/pci/controller/dwc/pci-keystone.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/pci/controller/dwc/pci-keystone.c
b/drivers/pci/controller/dwc/pci-keystone.c
index 5ffc3b40c4f6..00279002102e 100644
--- a/drivers/pci/controller/dwc/pci-keystone.c
+++ b/drivers/pci/controller/dwc/pci-keystone.c
@@ -1228,8 +1228,8 @@ static int __init ks_pcie_probe(struct platform_device
*pdev)
if (!pci)
return -ENOMEM;
- ks_pcie->va_app_base =
- devm_platform_ioremap_resource_byname(pdev, "app");
+ res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "app");
+ ks_pcie->va_app_base = devm_ioremap_resource(dev, res);
if (IS_ERR(ks_pcie->va_app_base))
return PTR_ERR(ks_pcie->va_app_base);
--
2.25.0
Re: [PATCH v1] PCI: controller: Remove duplicate error message
On Mon, Jul 06, 2020 at 04:58:47PM +0100, Lorenzo Pieralisi wrote: > On Tue, Jun 02, 2020 at 09:01:13AM -0600, Rob Herring wrote: > > [...] > > > > The other 2 error cases as well don't print the resource name as far as > > > I recall (they will at least print the resource start/end). > > > > Start/end are what are important for why either of these functions > > failed. > > > > But sure, we could add 'name' here. That's a separate patch IMO. Hi Lorenzo, Bob and Jonathan: Thank you very much for helping me review this patch, I sent a new patch for print the resource name when the request memory region or remapping of configuration space fails. and it is here: https://patchwork.kernel.org/patch/11657801/ BR, Dejin > > I agree. In sum, I think it is OK to proceed with this patch, provided > we send follow-ups as discussed here, are we in agreement ? > > Lorenzo
[PATCH v1] PCI: Add more information when devm_request_mem_region() goes wrong
Add more information, the name of the resource will be printed when the
request memory region or remapping of configuration space fails in the
devm_request_mem_region() function.
Signed-off-by: Dejin Zheng
---
drivers/pci/pci.c | 6 --
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index ce096272f52b..81c1045a3fa8 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -4153,13 +4153,15 @@ void __iomem *devm_pci_remap_cfg_resource(struct device
*dev,
name = res->name ?: dev_name(dev);
if (!devm_request_mem_region(dev, res->start, size, name)) {
- dev_err(dev, "can't request region for resource %pR\n", res);
+ dev_err(dev, "can't request region for %s's resource %pR\n",
+ name, res);
return IOMEM_ERR_PTR(-EBUSY);
}
dest_ptr = devm_pci_remap_cfgspace(dev, res->start, size);
if (!dest_ptr) {
- dev_err(dev, "ioremap failed for resource %pR\n", res);
+ dev_err(dev, "ioremap failed for %s's resource %pR\n",
+ name, res);
devm_release_mem_region(dev, res->start, size);
dest_ptr = IOMEM_ERR_PTR(-ENOMEM);
}
--
2.25.0
[PATCH v2] PCI: dwc: convert to devm_platform_ioremap_resource_byname()
Use devm_platform_ioremap_resource_byname() to simplify codes.
it contains platform_get_resource_byname() and devm_ioremap_resource().
Signed-off-by: Dejin Zheng
Reviewed-by: Gustavo Pimentel
Reviewed-by: Rob Herring
---
v1 -> v2:
- rebase to pci/dwc branch
- add Gustavo and Rob's Reviewed tag
drivers/pci/controller/dwc/pci-dra7xx.c | 11 ---
drivers/pci/controller/dwc/pci-keystone.c | 7 +++
drivers/pci/controller/dwc/pcie-artpec6.c | 12
.../pci/controller/dwc/pcie-designware-plat.c | 3 +--
drivers/pci/controller/dwc/pcie-histb.c | 7 ++-
drivers/pci/controller/dwc/pcie-intel-gw.c | 7 ++-
drivers/pci/controller/dwc/pcie-kirin.c | 17 ++---
drivers/pci/controller/dwc/pcie-qcom.c | 6 ++
drivers/pci/controller/dwc/pcie-uniphier.c | 3 +--
9 files changed, 25 insertions(+), 48 deletions(-)
diff --git a/drivers/pci/controller/dwc/pci-dra7xx.c
b/drivers/pci/controller/dwc/pci-dra7xx.c
index 6184ebc9392d..e5d0c7ac09b9 100644
--- a/drivers/pci/controller/dwc/pci-dra7xx.c
+++ b/drivers/pci/controller/dwc/pci-dra7xx.c
@@ -593,13 +593,12 @@ static int __init dra7xx_add_pcie_ep(struct dra7xx_pcie
*dra7xx,
ep = >ep;
ep->ops = _ep_ops;
- res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "ep_dbics");
- pci->dbi_base = devm_ioremap_resource(dev, res);
+ pci->dbi_base = devm_platform_ioremap_resource_byname(pdev, "ep_dbics");
if (IS_ERR(pci->dbi_base))
return PTR_ERR(pci->dbi_base);
- res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "ep_dbics2");
- pci->dbi_base2 = devm_ioremap_resource(dev, res);
+ pci->dbi_base2 =
+ devm_platform_ioremap_resource_byname(pdev, "ep_dbics2");
if (IS_ERR(pci->dbi_base2))
return PTR_ERR(pci->dbi_base2);
@@ -626,7 +625,6 @@ static int __init dra7xx_add_pcie_port(struct dra7xx_pcie
*dra7xx,
struct dw_pcie *pci = dra7xx->pci;
struct pcie_port *pp = >pp;
struct device *dev = pci->dev;
- struct resource *res;
pp->irq = platform_get_irq(pdev, 1);
if (pp->irq < 0) {
@@ -638,8 +636,7 @@ static int __init dra7xx_add_pcie_port(struct dra7xx_pcie
*dra7xx,
if (ret < 0)
return ret;
- res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "rc_dbics");
- pci->dbi_base = devm_ioremap_resource(dev, res);
+ pci->dbi_base = devm_platform_ioremap_resource_byname(pdev, "rc_dbics");
if (IS_ERR(pci->dbi_base))
return PTR_ERR(pci->dbi_base);
diff --git a/drivers/pci/controller/dwc/pci-keystone.c
b/drivers/pci/controller/dwc/pci-keystone.c
index 790679fdfa48..5ffc3b40c4f6 100644
--- a/drivers/pci/controller/dwc/pci-keystone.c
+++ b/drivers/pci/controller/dwc/pci-keystone.c
@@ -1228,8 +1228,8 @@ static int __init ks_pcie_probe(struct platform_device
*pdev)
if (!pci)
return -ENOMEM;
- res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "app");
- ks_pcie->va_app_base = devm_ioremap_resource(dev, res);
+ ks_pcie->va_app_base =
+ devm_platform_ioremap_resource_byname(pdev, "app");
if (IS_ERR(ks_pcie->va_app_base))
return PTR_ERR(ks_pcie->va_app_base);
@@ -1323,8 +1323,7 @@ static int __init ks_pcie_probe(struct platform_device
*pdev)
}
if (pci->version >= 0x480A) {
- res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "atu");
- atu_base = devm_ioremap_resource(dev, res);
+ atu_base = devm_platform_ioremap_resource_byname(pdev, "atu");
if (IS_ERR(atu_base)) {
ret = PTR_ERR(atu_base);
goto err_get_sync;
diff --git a/drivers/pci/controller/dwc/pcie-artpec6.c
b/drivers/pci/controller/dwc/pcie-artpec6.c
index 28d5a1095200..7d2cfa288b01 100644
--- a/drivers/pci/controller/dwc/pcie-artpec6.c
+++ b/drivers/pci/controller/dwc/pcie-artpec6.c
@@ -455,8 +455,7 @@ static int artpec6_add_pcie_ep(struct artpec6_pcie
*artpec6_pcie,
ep = >ep;
ep->ops = _ep_ops;
- res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "dbi2");
- pci->dbi_base2 = devm_ioremap_resource(dev, res);
+ pci->dbi_base2 = devm_platform_ioremap_resource_byname(pdev, "dbi2");
if (IS_ERR(pci->dbi_base2))
return PTR_ERR(pci->dbi_base2);
@@ -481,8 +480,6 @@ static int artpec6_pcie_probe(struct platform_device *pdev)
struct device *dev = >dev;
struct dw_pcie *pci;
struct artpec6_pcie *artpec6_pcie;
- struct resource *dbi_base;
- struct resou
Re: [PATCH v1] PCI: controller: convert to devm_platform_ioremap_resource()
On Tue, Jul 07, 2020 at 02:37:07PM +0100, Lorenzo Pieralisi wrote:
> On Wed, May 27, 2020 at 12:01:10AM +0800, Dejin Zheng wrote:
> > use devm_platform_ioremap_resource() to simplify code, it
> > contains platform_get_resource() and devm_ioremap_resource().
> >
> > Signed-off-by: Dejin Zheng
> > ---
> > drivers/pci/controller/dwc/pci-exynos.c | 4 +---
> > drivers/pci/controller/pci-aardvark.c | 5 ++---
> > drivers/pci/controller/pci-ftpci100.c | 4 +---
> > drivers/pci/controller/pci-versatile.c | 6 ++
> > drivers/pci/controller/pcie-brcmstb.c | 4 +---
> > 5 files changed, 7 insertions(+), 16 deletions(-)
>
> Can you rebase it please against:
>
> git://git.kernel.org/pub/scm/linux/kernel/git/lpieralisi/pci.git pci/misc
>
> I will apply it then (please carry over the review tags).
>
Hi Lorenzo:
I have sent the patch v2 for rebase it, Thank you very much!
The link is here:
https://patchwork.ozlabs.org/project/linux-pci/patch/20200708155614.308-1-zhengdej...@gmail.com/
BR,
Dejin
> Lorenzo
>
> > diff --git a/drivers/pci/controller/dwc/pci-exynos.c
> > b/drivers/pci/controller/dwc/pci-exynos.c
> > index c5043d951e80..5791039d6a54 100644
> > --- a/drivers/pci/controller/dwc/pci-exynos.c
> > +++ b/drivers/pci/controller/dwc/pci-exynos.c
> > @@ -84,14 +84,12 @@ static int exynos5440_pcie_get_mem_resources(struct
> > platform_device *pdev,
> > {
> > struct dw_pcie *pci = ep->pci;
> > struct device *dev = pci->dev;
> > - struct resource *res;
> >
> > ep->mem_res = devm_kzalloc(dev, sizeof(*ep->mem_res), GFP_KERNEL);
> > if (!ep->mem_res)
> > return -ENOMEM;
> >
> > - res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> > - ep->mem_res->elbi_base = devm_ioremap_resource(dev, res);
> > + ep->mem_res->elbi_base = devm_platform_ioremap_resource(pdev, 0);
> > if (IS_ERR(ep->mem_res->elbi_base))
> > return PTR_ERR(ep->mem_res->elbi_base);
> >
> > diff --git a/drivers/pci/controller/pci-aardvark.c
> > b/drivers/pci/controller/pci-aardvark.c
> > index 90ff291c24f0..0d98f9b04daa 100644
> > --- a/drivers/pci/controller/pci-aardvark.c
> > +++ b/drivers/pci/controller/pci-aardvark.c
> > @@ -1105,7 +1105,7 @@ static int advk_pcie_probe(struct platform_device
> > *pdev)
> > {
> > struct device *dev = >dev;
> > struct advk_pcie *pcie;
> > - struct resource *res, *bus;
> > + struct resource *bus;
> > struct pci_host_bridge *bridge;
> > int ret, irq;
> >
> > @@ -1116,8 +1116,7 @@ static int advk_pcie_probe(struct platform_device
> > *pdev)
> > pcie = pci_host_bridge_priv(bridge);
> > pcie->pdev = pdev;
> >
> > - res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> > - pcie->base = devm_ioremap_resource(dev, res);
> > + pcie->base = devm_platform_ioremap_resource(pdev, 0);
> > if (IS_ERR(pcie->base))
> > return PTR_ERR(pcie->base);
> >
> > diff --git a/drivers/pci/controller/pci-ftpci100.c
> > b/drivers/pci/controller/pci-ftpci100.c
> > index 1b67564de7af..221dfc9dc81b 100644
> > --- a/drivers/pci/controller/pci-ftpci100.c
> > +++ b/drivers/pci/controller/pci-ftpci100.c
> > @@ -422,7 +422,6 @@ static int faraday_pci_probe(struct platform_device
> > *pdev)
> > struct device *dev = >dev;
> > const struct faraday_pci_variant *variant =
> > of_device_get_match_data(dev);
> > - struct resource *regs;
> > struct resource_entry *win;
> > struct faraday_pci *p;
> > struct resource *io;
> > @@ -465,8 +464,7 @@ static int faraday_pci_probe(struct platform_device
> > *pdev)
> > return ret;
> > }
> >
> > - regs = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> > - p->base = devm_ioremap_resource(dev, regs);
> > + p->base = devm_platform_ioremap_resource(pdev, 0);
> > if (IS_ERR(p->base))
> > return PTR_ERR(p->base);
> >
> > diff --git a/drivers/pci/controller/pci-versatile.c
> > b/drivers/pci/controller/pci-versatile.c
> > index b911359b6d81..b34bbfe611e7 100644
> > --- a/drivers/pci/controller/pci-versatile.c
> > +++ b/drivers/pci/controller/pci-versatile.c
> > @@ -77,13 +77,11 @@ static int versatile_pci_probe(struct platform_device
> > *pdev)
> > if (!bridge)
> > return -ENOMEM;
> >
> > - res = platform_get_resource(pdev, IORE
[PATCH v2] PCI: controller: convert to devm_platform_ioremap_resource()
use devm_platform_ioremap_resource() to simplify code, it
contains platform_get_resource() and devm_ioremap_resource().
Signed-off-by: Dejin Zheng
Reviewed-by: Rob Herring
---
v1 -> v2:
- rebase to pci/misc branch
- add Rob Reviewed tag and Thanks for Bob's help.
drivers/pci/controller/dwc/pci-exynos.c | 4 +---
drivers/pci/controller/pci-aardvark.c | 5 ++---
drivers/pci/controller/pci-ftpci100.c | 4 +---
drivers/pci/controller/pci-versatile.c | 6 ++
drivers/pci/controller/pcie-brcmstb.c | 4 +---
5 files changed, 7 insertions(+), 16 deletions(-)
diff --git a/drivers/pci/controller/dwc/pci-exynos.c
b/drivers/pci/controller/dwc/pci-exynos.c
index c5043d951e80..5791039d6a54 100644
--- a/drivers/pci/controller/dwc/pci-exynos.c
+++ b/drivers/pci/controller/dwc/pci-exynos.c
@@ -84,14 +84,12 @@ static int exynos5440_pcie_get_mem_resources(struct
platform_device *pdev,
{
struct dw_pcie *pci = ep->pci;
struct device *dev = pci->dev;
- struct resource *res;
ep->mem_res = devm_kzalloc(dev, sizeof(*ep->mem_res), GFP_KERNEL);
if (!ep->mem_res)
return -ENOMEM;
- res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
- ep->mem_res->elbi_base = devm_ioremap_resource(dev, res);
+ ep->mem_res->elbi_base = devm_platform_ioremap_resource(pdev, 0);
if (IS_ERR(ep->mem_res->elbi_base))
return PTR_ERR(ep->mem_res->elbi_base);
diff --git a/drivers/pci/controller/pci-aardvark.c
b/drivers/pci/controller/pci-aardvark.c
index 90ff291c24f0..0d98f9b04daa 100644
--- a/drivers/pci/controller/pci-aardvark.c
+++ b/drivers/pci/controller/pci-aardvark.c
@@ -1105,7 +1105,7 @@ static int advk_pcie_probe(struct platform_device *pdev)
{
struct device *dev = >dev;
struct advk_pcie *pcie;
- struct resource *res, *bus;
+ struct resource *bus;
struct pci_host_bridge *bridge;
int ret, irq;
@@ -1116,8 +1116,7 @@ static int advk_pcie_probe(struct platform_device *pdev)
pcie = pci_host_bridge_priv(bridge);
pcie->pdev = pdev;
- res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
- pcie->base = devm_ioremap_resource(dev, res);
+ pcie->base = devm_platform_ioremap_resource(pdev, 0);
if (IS_ERR(pcie->base))
return PTR_ERR(pcie->base);
diff --git a/drivers/pci/controller/pci-ftpci100.c
b/drivers/pci/controller/pci-ftpci100.c
index 1b67564de7af..221dfc9dc81b 100644
--- a/drivers/pci/controller/pci-ftpci100.c
+++ b/drivers/pci/controller/pci-ftpci100.c
@@ -422,7 +422,6 @@ static int faraday_pci_probe(struct platform_device *pdev)
struct device *dev = >dev;
const struct faraday_pci_variant *variant =
of_device_get_match_data(dev);
- struct resource *regs;
struct resource_entry *win;
struct faraday_pci *p;
struct resource *io;
@@ -465,8 +464,7 @@ static int faraday_pci_probe(struct platform_device *pdev)
return ret;
}
- regs = platform_get_resource(pdev, IORESOURCE_MEM, 0);
- p->base = devm_ioremap_resource(dev, regs);
+ p->base = devm_platform_ioremap_resource(pdev, 0);
if (IS_ERR(p->base))
return PTR_ERR(p->base);
diff --git a/drivers/pci/controller/pci-versatile.c
b/drivers/pci/controller/pci-versatile.c
index e90f0cc65c73..a8d361f6c5d9 100644
--- a/drivers/pci/controller/pci-versatile.c
+++ b/drivers/pci/controller/pci-versatile.c
@@ -76,13 +76,11 @@ static int versatile_pci_probe(struct platform_device *pdev)
if (!bridge)
return -ENOMEM;
- res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
- versatile_pci_base = devm_ioremap_resource(dev, res);
+ versatile_pci_base = devm_platform_ioremap_resource(pdev, 0);
if (IS_ERR(versatile_pci_base))
return PTR_ERR(versatile_pci_base);
- res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
- versatile_cfg_base[0] = devm_ioremap_resource(dev, res);
+ versatile_cfg_base[0] = devm_platform_ioremap_resource(pdev, 1);
if (IS_ERR(versatile_cfg_base[0]))
return PTR_ERR(versatile_cfg_base[0]);
diff --git a/drivers/pci/controller/pcie-brcmstb.c
b/drivers/pci/controller/pcie-brcmstb.c
index 15c747c1390a..91a4b7f3ee45 100644
--- a/drivers/pci/controller/pcie-brcmstb.c
+++ b/drivers/pci/controller/pcie-brcmstb.c
@@ -933,7 +933,6 @@ static int brcm_pcie_probe(struct platform_device *pdev)
struct pci_host_bridge *bridge;
struct device_node *fw_np;
struct brcm_pcie *pcie;
- struct resource *res;
int ret;
/*
@@ -958,8 +957,7 @@ static int brcm_pcie_probe(struct platform_device *pdev)
pcie->dev = >dev;
pcie->np = np;
- res = platform_get_resource(pdev, IORESOURC
Re: [PATCH v1] reset: intel: fix a compile warning about REG_OFFSET redefined
On Fri, Jun 26, 2020 at 12:38:13PM +0200, Philipp Zabel wrote:
Hi Philipp:
Thanks very much for your review!
> Hi Dejin,
>
> On Thu, 2020-06-04 at 23:30 +0800, Dejin Zheng wrote:
> > kernel test robot reports a compile warning about REG_OFFSET redefined
> > in the reset-intel-gw.c after merging commit e44ab4e14d6f4 ("regmap:
> > Simplify implementation of the regmap_read_poll_timeout() macro"). the
> > warning is like that:
> >
> > drivers/reset/reset-intel-gw.c:18:0: warning: "REG_OFFSET" redefined
> > #define REG_OFFSET GENMASK(31, 16)
> >
> > In file included from ./arch/arm/mach-ixp4xx/include/mach/hardware.h:30:0,
> > from ./arch/arm/mach-ixp4xx/include/mach/io.h:15,
> > from ./arch/arm/include/asm/io.h:198,
> > from ./include/linux/io.h:13,
> > from ./include/linux/iopoll.h:14,
> > from ./include/linux/regmap.h:20,
> > from drivers/reset/reset-intel-gw.c:12:
> > ./arch/arm/mach-ixp4xx/include/mach/platform.h:25:0: note: this is the
> > location of the previous definition
> > #define REG_OFFSET 3
> >
> > Reported-by: kernel test robot
> > Fixes: e44ab4e14d6f4 ("regmap: Simplify implementation of the
> > regmap_read_poll_timeout() macro")
>
> Hm, shouldn't this rather be:
>
> Fixes: c9aef213e38c ("reset: intel: Add system reset controller driver")
>
> even though e44ab4e14d6f4 triggered the issue by including iopoll.h?
>
Ok.
> > Signed-off-by: Dejin Zheng
> > ---
> > drivers/reset/reset-intel-gw.c | 8
> > 1 file changed, 4 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/reset/reset-intel-gw.c b/drivers/reset/reset-intel-gw.c
> > index 854238444616..5cfb4892b399 100644
> > --- a/drivers/reset/reset-intel-gw.c
> > +++ b/drivers/reset/reset-intel-gw.c
> > @@ -15,7 +15,7 @@
> > #define RCU_RST_STAT 0x0024
> > #define RCU_RST_REQ0x0048
> >
> > -#define REG_OFFSET GENMASK(31, 16)
> > +#define REG_OFFSET_MASKGENMASK(31, 16)
> > #define BIT_OFFSET GENMASK(15, 8)
> > #define STAT_BIT_OFFSETGENMASK(7, 0)
>
> Could you add the _MASK suffix to BIT_OFFSET and STAT_BIT_OFFSET as
> well, for consistency?
>
I will add it in next patch version. Thanks!
BR,
Dejin
> regards
> Philipp
[PATCH net v3] net: phy: smsc: fix printing too many logs
Commit 7ae7ad2f11ef47 ("net: phy: smsc: use phy_read_poll_timeout()
to simplify the code") will print a lot of logs as follows when Ethernet
cable is not connected:
[4.473105] SMSC LAN8710/LAN8720 2188000.ethernet-1:00: lan87xx_read_status
failed: -110
When wait 640 ms for check ENERGYON bit, the timeout should not be
regarded as an actual error and an error message also should not be
printed. due to a hardware bug in LAN87XX device, it leads to unstable
detection of plugging in Ethernet cable when LAN87xx is in Energy Detect
Power-Down mode. the workaround for it involves, when the link is down,
and at each read_status() call:
- disable EDPD mode, forcing the PHY out of low-power mode
- waiting 640ms to see if we have any energy detected from the media
- re-enable entry to EDPD mode
This is presumably enough to allow the PHY to notice that a cable is
connected, and resume normal operations to negotiate with the partner.
The problem is that when no media is detected, the 640ms wait times
out and this commit was modified to prints an error message. it is an
inappropriate conversion by used phy_read_poll_timeout() to introduce
this bug. so fix this issue by use read_poll_timeout() to replace
phy_read_poll_timeout().
Fixes: 7ae7ad2f11ef47 ("net: phy: smsc: use phy_read_poll_timeout() to simplify
the code")
Reported-by: Kevin Groeneveld
Signed-off-by: Dejin Zheng
---
v2 -> v3:
- modify commit message to why need this patch. And many thanks
to Andrew and Russell for their comments.
v1 -> v2:
- add more commit message spell out what the change does.
drivers/net/phy/smsc.c | 11 +++
1 file changed, 7 insertions(+), 4 deletions(-)
diff --git a/drivers/net/phy/smsc.c b/drivers/net/phy/smsc.c
index 93da7d3d0954..74568ae16125 100644
--- a/drivers/net/phy/smsc.c
+++ b/drivers/net/phy/smsc.c
@@ -122,10 +122,13 @@ static int lan87xx_read_status(struct phy_device *phydev)
if (rc < 0)
return rc;
- /* Wait max 640 ms to detect energy */
- phy_read_poll_timeout(phydev, MII_LAN83C185_CTRL_STATUS, rc,
- rc & MII_LAN83C185_ENERGYON, 1,
- 64, true);
+ /* Wait max 640 ms to detect energy and the timeout is not
+* an actual error.
+*/
+ read_poll_timeout(phy_read, rc,
+ rc & MII_LAN83C185_ENERGYON || rc < 0,
+ 1, 64, true, phydev,
+ MII_LAN83C185_CTRL_STATUS);
if (rc < 0)
return rc;
--
2.25.0
Re: [PATCH net v1] net: phy: smsc: fix printing too many logs
On Wed, Jun 17, 2020 at 11:36:42PM +0200, Andrew Lunn wrote:
> On Wed, Jun 17, 2020 at 09:24:50PM +0100, Russell King - ARM Linux admin
> wrote:
> > On Wed, Jun 17, 2020 at 08:43:34PM +0200, Andrew Lunn wrote:
> > > You have explained what the change does. But not why it is
> > > needed. What exactly is happening. To me, the key thing is
> > > understanding why we get -110, and why it is not an actual error we
> > > should be reporting as an error. That is what needs explaining.
> >
> > The patch author really ought to be explaining this... but let me
> > have a go. It's worth pointing out that the comments in the file
> > aren't good English either, so don't really describe what is going
> > on.
> >
> > When this PHY is in EDPD mode, it doesn't always detect a connected
> > cable. The workaround for it involves, when the link is down, and
> > at each read_status() call:
> >
> > - disable EDPD mode, forcing the PHY out of low-power mode
> > - waiting 640ms to see if we have any energy detected from the media
> > - re-enable entry to EDPD mode
> >
> > This is presumably enough to allow the PHY to notice that a cable is
> > connected, and resume normal operations to negotiate with the partner.
> >
> > The problem is that when no media is detected, the 640ms wait times
> > out (as it should, we don't want to wait forever) and the kernel
> > prints a warning.
>
> Hi Russell
>
> Yes, that is what i was thinking.
>
> There probably should be a comment added just to prevent somebody
> swapping it back to phy_read_poll_timeout(). It is not clear that
> -ETIMEOUT is expected under some conditions.
>
> Andrew
Hi Andrew and Russell,
First of all, thanks very much for your comment!
I did not understand Andrew's comments correctly in the previous email,
sorry for my bad English. I found something in the commit 776829de90c597
("net: phy: workaround for buggy cable detection by LAN8700 after cable
plugging") about why it is not an actual error when get a timeout error
in this driver. the commit's link is here:
https://lore.kernel.org/patchwork/patch/590285/
As Russell said, it just for fix a hardware bug on LAN8700 device by wait
640ms, so it should not as an actual error when got timeout.
The following is my understanding:
It leads to unstable detection of plugging in Ethernet cable when LAN87xx
is in Energy Detect Power-Down mode. Because the ENERGYON bit is not set.
For fix this issue, it will disables Energy Detect Power-Down mode and
check ENERGYON bit to waiting for response on link pulses to detect
presence of plugged Ethernet cable in the 640ms. if got the ENERGYON bit
is set, I guess the cable plug-in event was detected and will report an
interrupt after exit lan87xx_read_status() funtion, if the ENERGYON bit
is not set, the plug-in event was not detected. after that, entry Energy
Detect Power-Down mode again.
it will re-call lan87xx_read_status() funtion by interrupt when got the
ENERGYON bit is set. and check link status again. then, It will get a
stable link status for LAN87xx device.
So the timeout for wait 640ms should not as an actual error.
Thanks!
Dejin
[PATCH net v2] net: phy: smsc: fix printing too many logs
Commit 7ae7ad2f11ef47 ("net: phy: smsc: use phy_read_poll_timeout()
to simplify the code") will print a lot of logs as follows when Ethernet
cable is not connected:
[4.473105] SMSC LAN8710/LAN8720 2188000.ethernet-1:00: lan87xx_read_status
failed: -110
The commit will change the original behavior in smsc driver.
It does not has any error message whether it is timeout or
phy_read() fails, but this commit will changed it and print some
error messages by phy_read_poll_timeout() when it is timeout or
phy_read() fails. so use the read_poll_timeout() function to replace
phy_read_poll_timeout() for fix this issue. the read_poll_timeout()
function does not print any log when it goes wrong.
the original codes is that:
/* Wait max 640 ms to detect energy */
for (i = 0; i < 64; i++) {
/* Sleep to allow link test pulses to be sent */
msleep(10);
rc = phy_read(phydev, MII_LAN83C185_CTRL_STATUS);
if (rc < 0)
return rc;
if (rc & MII_LAN83C185_ENERGYON)
break;
}
the commit 7ae7ad2f11ef47 ("net: phy: smsc: use phy_read_poll_timeout()
to simplify the code") modify it as this:
phy_read_poll_timeout(phydev, MII_LAN83C185_CTRL_STATUS,
rc & MII_LAN83C185_ENERGYON, 1,
64, true);
if (rc < 0)
return rc;
the phy_read_poll_timeout() will print a error log by phydev_err()
when timeout or rc < 0. read_poll_timeout() just return timeout
error and does not print any error log.
#define phy_read_poll_timeout(phydev, regnum, val, cond, sleep_us, \
timeout_us, sleep_before_read) \
({ \
int __ret = read_poll_timeout(phy_read, val, (cond) || val < 0, \
sleep_us, timeout_us, sleep_before_read, phydev, regnum); \
if (val < 0) \
__ret = val; \
if (__ret) \
phydev_err(phydev, "%s failed: %d\n", __func__, __ret); \
__ret; \
})
So use read_poll_timeout() to replace phy_read_poll_timeout() for
be consistent with the original code and fix this issue.
Fixes: 7ae7ad2f11ef47 ("net: phy: smsc: use phy_read_poll_timeout() to simplify
the code")
Reported-by: Kevin Groeneveld
Signed-off-by: Dejin Zheng
---
v1 -> v2:
- add more commit message spell out why has this commit
and how to modify it.
drivers/net/phy/smsc.c | 7 ---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/drivers/net/phy/smsc.c b/drivers/net/phy/smsc.c
index 93da7d3d0954..36c5a57917b8 100644
--- a/drivers/net/phy/smsc.c
+++ b/drivers/net/phy/smsc.c
@@ -123,9 +123,10 @@ static int lan87xx_read_status(struct phy_device *phydev)
return rc;
/* Wait max 640 ms to detect energy */
- phy_read_poll_timeout(phydev, MII_LAN83C185_CTRL_STATUS, rc,
- rc & MII_LAN83C185_ENERGYON, 1,
- 64, true);
+ read_poll_timeout(phy_read, rc,
+ rc & MII_LAN83C185_ENERGYON || rc < 0,
+ 1, 64, true, phydev,
+ MII_LAN83C185_CTRL_STATUS);
if (rc < 0)
return rc;
--
2.25.0
Re: [PATCH net v1] net: phy: smsc: fix printing too many logs
On Wed, Jun 17, 2020 at 06:19:25PM +0200, Andrew Lunn wrote:
> On Wed, Jun 17, 2020 at 11:33:40PM +0800, Dejin Zheng wrote:
> > Commit 7ae7ad2f11ef47 ("net: phy: smsc: use phy_read_poll_timeout()
> > to simplify the code") will print a lot of logs as follows when Ethernet
> > cable is not connected:
> >
> > [4.473105] SMSC LAN8710/LAN8720 2188000.ethernet-1:00:
> > lan87xx_read_status failed: -110
> >
> > So fix it by read_poll_timeout().
>
> Do you have a more detailed explanation of what is going on here?
>
> After a lot of thought, i think i can see how this happens. But the
> commit message should really spell out why this is the correct fix.
>
Hi Andrew:
Kevin report a bug for me in link[1], I check the Commit 7ae7ad2f11ef47
("net: phy: smsc: use phy_read_poll_timeout() to simplify the code") and
found it change the original behavior in smsc driver. It does not has
any error message whether it is timeout or phy_read fails, but this Commit
will changed it and will print some error messages by
phy_read_poll_timeout() when it is timeout or phy_read fails. so use the
read_poll_timeout() to replace phy_read_poll_timeout() to fix this
issue. the read_poll_timeout() does not print any log when it goes
wrong.
the original codes is that:
/* Wait max 640 ms to detect energy */
for (i = 0; i < 64; i++) {
/* Sleep to allow link test pulses to be sent */
msleep(10);
rc = phy_read(phydev, MII_LAN83C185_CTRL_STATUS);
if (rc < 0)
return rc;
if (rc & MII_LAN83C185_ENERGYON)
break;
}
Commit 7ae7ad2f11ef47 modify it as this:
phy_read_poll_timeout(phydev, MII_LAN83C185_CTRL_STATUS,
rc & MII_LAN83C185_ENERGYON, 1,
64, true);
if (rc < 0)
return rc;
the phy_read_poll_timeout() will print a error log by phydev_err()
when timeout or rc < 0. read_poll_timeout() just return timeout
error and does not print any error log.
#define phy_read_poll_timeout(phydev, regnum, val, cond, sleep_us, \
timeout_us, sleep_before_read) \
({ \
int __ret = read_poll_timeout(phy_read, val, (cond) || val < 0, \
sleep_us, timeout_us, sleep_before_read, phydev, regnum); \
if (val < 0) \
__ret = val; \
if (__ret) \
phydev_err(phydev, "%s failed: %d\n", __func__, __ret); \
__ret; \
})
So use read_poll_timeout Use read_poll_timeout() to be consistent with the
original code.
link[1]: https://lkml.org/lkml/2020/6/1/1408
> Thanks
> Andrew
[PATCH net v1] net: phy: smsc: fix printing too many logs
Commit 7ae7ad2f11ef47 ("net: phy: smsc: use phy_read_poll_timeout()
to simplify the code") will print a lot of logs as follows when Ethernet
cable is not connected:
[4.473105] SMSC LAN8710/LAN8720 2188000.ethernet-1:00: lan87xx_read_status
failed: -110
So fix it by read_poll_timeout().
Fixes: 7ae7ad2f11ef47 ("net: phy: smsc: use phy_read_poll_timeout() to simplify
the code")
Reported-by: Kevin Groeneveld
Signed-off-by: Dejin Zheng
---
drivers/net/phy/smsc.c | 7 ---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/drivers/net/phy/smsc.c b/drivers/net/phy/smsc.c
index 93da7d3d0954..36c5a57917b8 100644
--- a/drivers/net/phy/smsc.c
+++ b/drivers/net/phy/smsc.c
@@ -123,9 +123,10 @@ static int lan87xx_read_status(struct phy_device *phydev)
return rc;
/* Wait max 640 ms to detect energy */
- phy_read_poll_timeout(phydev, MII_LAN83C185_CTRL_STATUS, rc,
- rc & MII_LAN83C185_ENERGYON, 1,
- 64, true);
+ read_poll_timeout(phy_read, rc,
+ rc & MII_LAN83C185_ENERGYON || rc < 0,
+ 1, 64, true, phydev,
+ MII_LAN83C185_CTRL_STATUS);
if (rc < 0)
return rc;
--
2.25.0
Re: drivers/reset/reset-intel-gw.c:18:9: warning: 'REG_OFFSET' macro redefined
On Mon, Jun 15, 2020 at 10:33:12PM +0800, kernel test robot wrote: > tree: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git > master > head: b3a9e3b9622ae10064826dccb4f7a52bd88c7407 > commit: e44ab4e14d6f4c448ae555132090c1a116b19e5c regmap: Simplify > implementation of the regmap_read_poll_timeout() macro > date: 8 weeks ago > config: arm-randconfig-r022-20200615 (attached as .config) > compiler: clang version 11.0.0 (https://github.com/llvm/llvm-project > 3d8149c2a1228609fd7d7c91a04681304a2f0ca9) > reproduce (this is a W=1 build): > wget > https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O > ~/bin/make.cross > chmod +x ~/bin/make.cross > # install arm cross compiling tool for clang build > # apt-get install binutils-arm-linux-gnueabi > git checkout e44ab4e14d6f4c448ae555132090c1a116b19e5c > # save the attached .config to linux build tree > COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=arm > > If you fix the issue, kindly add following tag as appropriate > Reported-by: kernel test robot > Hi I sent a patch to fix it, and now the patch is currently under review. https://lkml.org/lkml/2020/6/4/606 BR, Dejin > All warnings (new ones prefixed by >>, old ones prefixed by <<): > > >> drivers/reset/reset-intel-gw.c:18:9: warning: 'REG_OFFSET' macro redefined > >> [-Wmacro-redefined] > #define REG_OFFSET GENMASK(31, 16) > ^ > arch/arm/mach-ixp4xx/include/mach/platform.h:25:9: note: previous definition > is here > #define REG_OFFSET 3 > ^ > 1 warning generated. > > vim +/REG_OFFSET +18 drivers/reset/reset-intel-gw.c > > c9aef213e38cde Dilip Kota 2020-01-03 17 > c9aef213e38cde Dilip Kota 2020-01-03 @18 #define REG_OFFSET GENMASK(31, 16) > c9aef213e38cde Dilip Kota 2020-01-03 19 #define BIT_OFFSET GENMASK(15, 8) > c9aef213e38cde Dilip Kota 2020-01-03 20 #define STAT_BIT_OFFSET > GENMASK(7, 0) > c9aef213e38cde Dilip Kota 2020-01-03 21 > > :: The code at line 18 was first introduced by commit > :: c9aef213e38cde27d4689a5cbe25a7c1b1db9fad reset: intel: Add system > reset controller driver > > :: TO: Dilip Kota > :: CC: Philipp Zabel > > --- > 0-DAY CI Kernel Test Service, Intel Corporation > https://lists.01.org/hyperkitty/list/kbuild-...@lists.01.org
Re: drivers/reset/reset-intel-gw.c:18: warning: "REG_OFFSET" redefined
On Thu, Jun 04, 2020 at 07:37:06PM +0800, kernel test robot wrote: > tree: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git > master > head: 6929f71e46bdddbf1c4d67c2728648176c67c555 > commit: e44ab4e14d6f4c448ae555132090c1a116b19e5c regmap: Simplify > implementation of the regmap_read_poll_timeout() macro > date: 6 weeks ago > config: arm-randconfig-r035-20200604 (attached as .config) > compiler: arm-linux-gnueabi-gcc (GCC) 9.3.0 > reproduce (this is a W=1 build): > wget > https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O > ~/bin/make.cross > chmod +x ~/bin/make.cross > git checkout e44ab4e14d6f4c448ae555132090c1a116b19e5c > # save the attached .config to linux build tree > COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross > ARCH=arm > > If you fix the issue, kindly add following tag as appropriate > Reported-by: kernel test robot > Hi Thanks very much for report this issue to me, and very very sorry for that. I submit a commit to fix it. it is in here: https://lkml.org/lkml/2020/6/4/606 BR, Dejin > All warnings (new ones prefixed by >>, old ones prefixed by <<): > > >> drivers/reset/reset-intel-gw.c:18: warning: "REG_OFFSET" redefined > 18 | #define REG_OFFSET GENMASK(31, 16) > | > In file included from arch/arm/mach-ixp4xx/include/mach/hardware.h:30, > from arch/arm/mach-ixp4xx/include/mach/io.h:15, > from arch/arm/include/asm/io.h:198, > from include/linux/io.h:13, > from include/linux/iopoll.h:14, > from include/linux/regmap.h:20, > from drivers/reset/reset-intel-gw.c:12: > arch/arm/mach-ixp4xx/include/mach/platform.h:25: note: this is the location > of the previous definition > 25 | #define REG_OFFSET 3 > | > > vim +/REG_OFFSET +18 drivers/reset/reset-intel-gw.c > > c9aef213e38cde Dilip Kota 2020-01-03 17 > c9aef213e38cde Dilip Kota 2020-01-03 @18 #define REG_OFFSET GENMASK(31, 16) > c9aef213e38cde Dilip Kota 2020-01-03 19 #define BIT_OFFSET GENMASK(15, 8) > c9aef213e38cde Dilip Kota 2020-01-03 20 #define STAT_BIT_OFFSET > GENMASK(7, 0) > c9aef213e38cde Dilip Kota 2020-01-03 21 > > :: The code at line 18 was first introduced by commit > :: c9aef213e38cde27d4689a5cbe25a7c1b1db9fad reset: intel: Add system > reset controller driver > > :: TO: Dilip Kota > :: CC: Philipp Zabel > > --- > 0-DAY CI Kernel Test Service, Intel Corporation > https://lists.01.org/hyperkitty/list/kbuild-...@lists.01.org
[PATCH v1] reset: intel: fix a compile warning about REG_OFFSET redefined
kernel test robot reports a compile warning about REG_OFFSET redefined
in the reset-intel-gw.c after merging commit e44ab4e14d6f4 ("regmap:
Simplify implementation of the regmap_read_poll_timeout() macro"). the
warning is like that:
drivers/reset/reset-intel-gw.c:18:0: warning: "REG_OFFSET" redefined
#define REG_OFFSET GENMASK(31, 16)
In file included from ./arch/arm/mach-ixp4xx/include/mach/hardware.h:30:0,
from ./arch/arm/mach-ixp4xx/include/mach/io.h:15,
from ./arch/arm/include/asm/io.h:198,
from ./include/linux/io.h:13,
from ./include/linux/iopoll.h:14,
from ./include/linux/regmap.h:20,
from drivers/reset/reset-intel-gw.c:12:
./arch/arm/mach-ixp4xx/include/mach/platform.h:25:0: note: this is the location
of the previous definition
#define REG_OFFSET 3
Reported-by: kernel test robot
Fixes: e44ab4e14d6f4 ("regmap: Simplify implementation of the
regmap_read_poll_timeout() macro")
Signed-off-by: Dejin Zheng
---
drivers/reset/reset-intel-gw.c | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/reset/reset-intel-gw.c b/drivers/reset/reset-intel-gw.c
index 854238444616..5cfb4892b399 100644
--- a/drivers/reset/reset-intel-gw.c
+++ b/drivers/reset/reset-intel-gw.c
@@ -15,7 +15,7 @@
#define RCU_RST_STAT 0x0024
#define RCU_RST_REQ0x0048
-#define REG_OFFSET GENMASK(31, 16)
+#define REG_OFFSET_MASKGENMASK(31, 16)
#define BIT_OFFSET GENMASK(15, 8)
#define STAT_BIT_OFFSETGENMASK(7, 0)
@@ -51,7 +51,7 @@ static u32 id_to_reg_and_bit_offsets(struct intel_reset_data
*data,
unsigned long id, u32 *rst_req,
u32 *req_bit, u32 *stat_bit)
{
- *rst_req = FIELD_GET(REG_OFFSET, id);
+ *rst_req = FIELD_GET(REG_OFFSET_MASK, id);
*req_bit = FIELD_GET(BIT_OFFSET, id);
if (data->soc_data->legacy)
@@ -141,7 +141,7 @@ static int intel_reset_xlate(struct reset_controller_dev
*rcdev,
if (spec->args[1] > 31)
return -EINVAL;
- id = FIELD_PREP(REG_OFFSET, spec->args[0]);
+ id = FIELD_PREP(REG_OFFSET_MASK, spec->args[0]);
id |= FIELD_PREP(BIT_OFFSET, spec->args[1]);
if (data->soc_data->legacy) {
@@ -210,7 +210,7 @@ static int intel_reset_probe(struct platform_device *pdev)
if (ret)
return ret;
- data->reboot_id = FIELD_PREP(REG_OFFSET, rb_id[0]);
+ data->reboot_id = FIELD_PREP(REG_OFFSET_MASK, rb_id[0]);
data->reboot_id |= FIELD_PREP(BIT_OFFSET, rb_id[1]);
if (data->soc_data->legacy)
--
2.25.0
[PATCH v2] PCI: controller: convert to devm_platform_ioremap_resource_byname()
Use devm_platform_ioremap_resource_byname() to simplify codes.
it contains platform_get_resource_byname() and devm_ioremap_resource().
Signed-off-by: Dejin Zheng
---
v1 -> v2:
- Discard changes to the file drivers/pci/controller/pcie-xilinx-nwl.c
Due to my mistakes, my patch will modify pcie-xilinx-nwl.c,
but it still need to use the res variable, but
devm_platform_ioremap_resource_byname() funtion can't assign a
value to the variable res. kbuild test robot report it. Thanks
very much for kbuild test robot .
drivers/pci/controller/cadence/pcie-cadence-ep.c | 3 +--
drivers/pci/controller/cadence/pcie-cadence-host.c | 3 +--
drivers/pci/controller/pci-tegra.c | 8 +++-
drivers/pci/controller/pci-xgene.c | 3 +--
drivers/pci/controller/pcie-altera-msi.c | 3 +--
drivers/pci/controller/pcie-altera.c | 9 +++--
drivers/pci/controller/pcie-mediatek.c | 4 +---
drivers/pci/controller/pcie-rockchip.c | 5 ++---
8 files changed, 13 insertions(+), 25 deletions(-)
diff --git a/drivers/pci/controller/cadence/pcie-cadence-ep.c
b/drivers/pci/controller/cadence/pcie-cadence-ep.c
index 1c15c8352125..74ffa03fde5f 100644
--- a/drivers/pci/controller/cadence/pcie-cadence-ep.c
+++ b/drivers/pci/controller/cadence/pcie-cadence-ep.c
@@ -408,8 +408,7 @@ int cdns_pcie_ep_setup(struct cdns_pcie_ep *ep)
pcie->is_rc = false;
- res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "reg");
- pcie->reg_base = devm_ioremap_resource(dev, res);
+ pcie->reg_base = devm_platform_ioremap_resource_byname(pdev, "reg");
if (IS_ERR(pcie->reg_base)) {
dev_err(dev, "missing \"reg\"\n");
return PTR_ERR(pcie->reg_base);
diff --git a/drivers/pci/controller/cadence/pcie-cadence-host.c
b/drivers/pci/controller/cadence/pcie-cadence-host.c
index 8c2543f28ba0..dcc460a54875 100644
--- a/drivers/pci/controller/cadence/pcie-cadence-host.c
+++ b/drivers/pci/controller/cadence/pcie-cadence-host.c
@@ -225,8 +225,7 @@ int cdns_pcie_host_setup(struct cdns_pcie_rc *rc)
rc->device_id = 0x;
of_property_read_u32(np, "device-id", >device_id);
- res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "reg");
- pcie->reg_base = devm_ioremap_resource(dev, res);
+ pcie->reg_base = devm_platform_ioremap_resource_byname(pdev, "reg");
if (IS_ERR(pcie->reg_base)) {
dev_err(dev, "missing \"reg\"\n");
return PTR_ERR(pcie->reg_base);
diff --git a/drivers/pci/controller/pci-tegra.c
b/drivers/pci/controller/pci-tegra.c
index e3e917243e10..3e608383df66 100644
--- a/drivers/pci/controller/pci-tegra.c
+++ b/drivers/pci/controller/pci-tegra.c
@@ -1462,7 +1462,7 @@ static int tegra_pcie_get_resources(struct tegra_pcie
*pcie)
{
struct device *dev = pcie->dev;
struct platform_device *pdev = to_platform_device(dev);
- struct resource *pads, *afi, *res;
+ struct resource *res;
const struct tegra_pcie_soc *soc = pcie->soc;
int err;
@@ -1486,15 +1486,13 @@ static int tegra_pcie_get_resources(struct tegra_pcie
*pcie)
}
}
- pads = platform_get_resource_byname(pdev, IORESOURCE_MEM, "pads");
- pcie->pads = devm_ioremap_resource(dev, pads);
+ pcie->pads = devm_platform_ioremap_resource_byname(pdev, "pads");
if (IS_ERR(pcie->pads)) {
err = PTR_ERR(pcie->pads);
goto phys_put;
}
- afi = platform_get_resource_byname(pdev, IORESOURCE_MEM, "afi");
- pcie->afi = devm_ioremap_resource(dev, afi);
+ pcie->afi = devm_platform_ioremap_resource_byname(pdev, "afi");
if (IS_ERR(pcie->afi)) {
err = PTR_ERR(pcie->afi);
goto phys_put;
diff --git a/drivers/pci/controller/pci-xgene.c
b/drivers/pci/controller/pci-xgene.c
index d1efa8ffbae1..1431a18eb02c 100644
--- a/drivers/pci/controller/pci-xgene.c
+++ b/drivers/pci/controller/pci-xgene.c
@@ -355,8 +355,7 @@ static int xgene_pcie_map_reg(struct xgene_pcie_port *port,
if (IS_ERR(port->csr_base))
return PTR_ERR(port->csr_base);
- res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "cfg");
- port->cfg_base = devm_ioremap_resource(dev, res);
+ port->cfg_base = devm_platform_ioremap_resource_byname(pdev, "cfg");
if (IS_ERR(port->cfg_base))
return PTR_ERR(port->cfg_base);
port->cfg_addr = res->start;
diff --git a/drivers/pci/controller/pcie-altera-msi.c
b/drivers/pci/controller/pcie-altera-msi.c
index 16d938920ca5..613e19af71bd 10064
Re: [PATCH v1] PCI: controller: convert to devm_platform_ioremap_resource_byname()
On Mon, Jun 01, 2020 at 04:22:29PM -0600, Rob Herring wrote:
> On Mon, Jun 01, 2020 at 10:33:45PM +0800, Dejin Zheng wrote:
> > Use devm_platform_ioremap_resource_byname() to simplify codes.
> > it contains platform_get_resource_byname() and devm_ioremap_resource().
> >
> > Signed-off-by: Dejin Zheng
> > ---
> > drivers/pci/controller/cadence/pcie-cadence-ep.c | 3 +--
> > drivers/pci/controller/cadence/pcie-cadence-host.c | 3 +--
> > drivers/pci/controller/pci-tegra.c | 8 +++-
> > drivers/pci/controller/pci-xgene.c | 3 +--
> > drivers/pci/controller/pcie-altera-msi.c | 3 +--
> > drivers/pci/controller/pcie-altera.c | 9 +++--
> > drivers/pci/controller/pcie-mediatek.c | 4 +---
> > drivers/pci/controller/pcie-rockchip.c | 5 ++---
> > drivers/pci/controller/pcie-xilinx-nwl.c | 7 +++
> > 9 files changed, 16 insertions(+), 29 deletions(-)
> >
> > diff --git a/drivers/pci/controller/cadence/pcie-cadence-ep.c
> > b/drivers/pci/controller/cadence/pcie-cadence-ep.c
> > index 1c15c8352125..74ffa03fde5f 100644
> > --- a/drivers/pci/controller/cadence/pcie-cadence-ep.c
> > +++ b/drivers/pci/controller/cadence/pcie-cadence-ep.c
> > @@ -408,8 +408,7 @@ int cdns_pcie_ep_setup(struct cdns_pcie_ep *ep)
> >
> > pcie->is_rc = false;
> >
> > - res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "reg");
> > - pcie->reg_base = devm_ioremap_resource(dev, res);
> > + pcie->reg_base = devm_platform_ioremap_resource_byname(pdev, "reg");
> > if (IS_ERR(pcie->reg_base)) {
> > dev_err(dev, "missing \"reg\"\n");
> > return PTR_ERR(pcie->reg_base);
> > diff --git a/drivers/pci/controller/cadence/pcie-cadence-host.c
> > b/drivers/pci/controller/cadence/pcie-cadence-host.c
> > index 8c2543f28ba0..dcc460a54875 100644
> > --- a/drivers/pci/controller/cadence/pcie-cadence-host.c
> > +++ b/drivers/pci/controller/cadence/pcie-cadence-host.c
> > @@ -225,8 +225,7 @@ int cdns_pcie_host_setup(struct cdns_pcie_rc *rc)
> > rc->device_id = 0x;
> > of_property_read_u32(np, "device-id", >device_id);
> >
> > - res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "reg");
> > - pcie->reg_base = devm_ioremap_resource(dev, res);
> > + pcie->reg_base = devm_platform_ioremap_resource_byname(pdev, "reg");
> > if (IS_ERR(pcie->reg_base)) {
> > dev_err(dev, "missing \"reg\"\n");
> > return PTR_ERR(pcie->reg_base);
> > diff --git a/drivers/pci/controller/pci-tegra.c
> > b/drivers/pci/controller/pci-tegra.c
> > index e3e917243e10..3e608383df66 100644
> > --- a/drivers/pci/controller/pci-tegra.c
> > +++ b/drivers/pci/controller/pci-tegra.c
> > @@ -1462,7 +1462,7 @@ static int tegra_pcie_get_resources(struct tegra_pcie
> > *pcie)
> > {
> > struct device *dev = pcie->dev;
> > struct platform_device *pdev = to_platform_device(dev);
> > - struct resource *pads, *afi, *res;
> > + struct resource *res;
> > const struct tegra_pcie_soc *soc = pcie->soc;
> > int err;
> >
> > @@ -1486,15 +1486,13 @@ static int tegra_pcie_get_resources(struct
> > tegra_pcie *pcie)
> > }
> > }
> >
> > - pads = platform_get_resource_byname(pdev, IORESOURCE_MEM, "pads");
> > - pcie->pads = devm_ioremap_resource(dev, pads);
> > + pcie->pads = devm_platform_ioremap_resource_byname(pdev, "pads");
> > if (IS_ERR(pcie->pads)) {
> > err = PTR_ERR(pcie->pads);
> > goto phys_put;
> > }
> >
> > - afi = platform_get_resource_byname(pdev, IORESOURCE_MEM, "afi");
> > - pcie->afi = devm_ioremap_resource(dev, afi);
> > + pcie->afi = devm_platform_ioremap_resource_byname(pdev, "afi");
> > if (IS_ERR(pcie->afi)) {
> > err = PTR_ERR(pcie->afi);
> > goto phys_put;
> > diff --git a/drivers/pci/controller/pci-xgene.c
> > b/drivers/pci/controller/pci-xgene.c
> > index d1efa8ffbae1..1431a18eb02c 100644
> > --- a/drivers/pci/controller/pci-xgene.c
> > +++ b/drivers/pci/controller/pci-xgene.c
> > @@ -355,8 +355,7 @@ static int xgene_pcie_map_reg(struct xgene_pcie_port
> > *port,
> > if (IS_ERR(port->csr_base))
> > return PTR_ERR(port->
Re: [PATCH v1] PCI: controller: convert to devm_platform_ioremap_resource_byname()
On Tue, Jun 02, 2020 at 02:13:23AM +0800, kbuild test robot wrote:
> Hi Dejin,
>
> Thank you for the patch! Perhaps something to improve:
>
Yes, you are right, I should not modify this file
drivers/pci/controller/pcie-xilinx-nwl.c.
I will sent the patch v2. Thanks very much!
BR,
Dejin
> [auto build test WARNING on pci/next]
> [also build test WARNING on tegra/for-next rockchip/for-next v5.7
> next-20200529]
> [if your patch is applied to the wrong git tree, please drop us a note to help
> improve the system. BTW, we also suggest to use '--base' option to specify the
> base tree in git format-patch, please see
> https://stackoverflow.com/a/37406982]
>
> url:
> https://github.com/0day-ci/linux/commits/Dejin-Zheng/PCI-controller-convert-to-devm_platform_ioremap_resource_byname/20200601-223757
> base: https://git.kernel.org/pub/scm/linux/kernel/git/helgaas/pci.git next
> config: x86_64-allyesconfig (attached as .config)
> compiler: clang version 11.0.0 (https://github.com/llvm/llvm-project
> 2388a096e7865c043e83ece4e26654bd3d1a20d5)
> reproduce (this is a W=1 build):
> wget
> https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O
> ~/bin/make.cross
> chmod +x ~/bin/make.cross
> # install x86_64 cross compiling tool for clang build
> # apt-get install binutils-x86-64-linux-gnu
> # save the attached .config to linux build tree
> COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross
> ARCH=x86_64
>
> If you fix the issue, kindly add following tag as appropriate
> Reported-by: kbuild test robot
>
> All warnings (new ones prefixed by >>, old ones prefixed by <<):
>
> >> drivers/pci/controller/pcie-xilinx-nwl.c:783:25: warning: variable 'res'
> >> is uninitialized when used here [-Wuninitialized]
> pcie->phys_breg_base = res->start;
> ^~~
> drivers/pci/controller/pcie-xilinx-nwl.c:778:22: note: initialize the
> variable 'res' to silence this warning
> struct resource *res;
> ^
> = NULL
> 1 warning generated.
>
> vim +/res +783 drivers/pci/controller/pcie-xilinx-nwl.c
>
> ab597d35ef11d2 drivers/pci/host/pcie-xilinx-nwl.c Bharat Kumar Gogada
> 2016-03-06 773
> ab597d35ef11d2 drivers/pci/host/pcie-xilinx-nwl.c Bharat Kumar Gogada
> 2016-03-06 774 static int nwl_pcie_parse_dt(struct nwl_pcie *pcie,
> ab597d35ef11d2 drivers/pci/host/pcie-xilinx-nwl.c Bharat Kumar Gogada
> 2016-03-06 775struct platform_device *pdev)
> ab597d35ef11d2 drivers/pci/host/pcie-xilinx-nwl.c Bharat Kumar Gogada
> 2016-03-06 776 {
> adf9e284b4f76d drivers/pci/host/pcie-xilinx-nwl.c Bjorn Helgaas
> 2016-10-06 777 struct device *dev = pcie->dev;
> ab597d35ef11d2 drivers/pci/host/pcie-xilinx-nwl.c Bharat Kumar Gogada
> 2016-03-06 778 struct resource *res;
> ab597d35ef11d2 drivers/pci/host/pcie-xilinx-nwl.c Bharat Kumar Gogada
> 2016-03-06 779
> 213cf573e1a8b2 drivers/pci/controller/pcie-xilinx-nwl.c Dejin Zheng
> 2020-06-01 780 pcie->breg_base =
> devm_platform_ioremap_resource_byname(pdev, "breg");
> ab597d35ef11d2 drivers/pci/host/pcie-xilinx-nwl.c Bharat Kumar Gogada
> 2016-03-06 781 if (IS_ERR(pcie->breg_base))
> ab597d35ef11d2 drivers/pci/host/pcie-xilinx-nwl.c Bharat Kumar Gogada
> 2016-03-06 782 return PTR_ERR(pcie->breg_base);
> ab597d35ef11d2 drivers/pci/host/pcie-xilinx-nwl.c Bharat Kumar Gogada
> 2016-03-06 @783 pcie->phys_breg_base = res->start;
> ab597d35ef11d2 drivers/pci/host/pcie-xilinx-nwl.c Bharat Kumar Gogada
> 2016-03-06 784
> 213cf573e1a8b2 drivers/pci/controller/pcie-xilinx-nwl.c Dejin Zheng
> 2020-06-01 785 pcie->pcireg_base =
> 213cf573e1a8b2 drivers/pci/controller/pcie-xilinx-nwl.c Dejin Zheng
> 2020-06-01 786 devm_platform_ioremap_resource_byname(pdev,
> "pcireg");
> ab597d35ef11d2 drivers/pci/host/pcie-xilinx-nwl.c Bharat Kumar Gogada
> 2016-03-06 787 if (IS_ERR(pcie->pcireg_base))
> ab597d35ef11d2 drivers/pci/host/pcie-xilinx-nwl.c Bharat Kumar Gogada
> 2016-03-06 788 return PTR_ERR(pcie->pcireg_base);
> ab597d35ef11d2 drivers/pci/host/pcie-xilinx-nwl.c Bharat Kumar Gogada
> 2016-03-06 789 pcie->phys_pcie_reg_base = res->start;
> ab597d35ef11d2 drivers/pci/host/pcie-xilinx-nwl.c Bharat Kumar Gogada
> 2016-03-06 790
> ab597d35ef11d2 drivers/pci/host/pcie-xilinx-nwl.c Bharat Kumar Gogada
> 2016-03-06 791 res = platform_get_resource_byname(pdev, IORESOURCE_MEM,
> "cfg");
> cd00f084ed1d50 drivers/pci/host/pcie-xilinx-nwl.
Re: [PATCH net-next v7 09/10] net: phy: smsc: use phy_read_poll_timeout() to simplify the code
On Mon, Jun 01, 2020 at 02:58:21PM -0400, Kevin Groeneveld wrote:
> On Mon, Mar 23, 2020 at 11:10 AM Dejin Zheng wrote:
> >
> > use phy_read_poll_timeout() to replace the poll codes for
> > simplify lan87xx_read_status() function.
> >
> > Suggested-by: Andrew Lunn
> > Reviewed-by: Florian Fainelli
> > Signed-off-by: Dejin Zheng
> > ---
> > v6 -> v7:
> > - adapt to a newly added parameter sleep_before_read.
> > v5 -> v6:
> > - no changed.
> > v4 -> v5:
> > - add msleep before phy_read_poll_timeout() to keep the
> > code more similar
> > v3 -> v4:
> > - add this patch by Andrew's suggestion. Thanks Andrew!
> >
> > drivers/net/phy/smsc.c | 16 +---
> > 1 file changed, 5 insertions(+), 11 deletions(-)
> >
> > diff --git a/drivers/net/phy/smsc.c b/drivers/net/phy/smsc.c
> > index b73298250793..93da7d3d0954 100644
> > --- a/drivers/net/phy/smsc.c
> > +++ b/drivers/net/phy/smsc.c
> > @@ -112,8 +112,6 @@ static int lan87xx_read_status(struct phy_device
> *phydev)
> > int err = genphy_read_status(phydev);
> >
> > if (!phydev->link && priv->energy_enable) {
> > - int i;
> > -
> > /* Disable EDPD to wake up PHY */
> > int rc = phy_read(phydev, MII_LAN83C185_CTRL_STATUS);
> > if (rc < 0)
> > @@ -125,15 +123,11 @@ static int lan87xx_read_status(struct phy_device
> *phydev)
> > return rc;
> >
> > /* Wait max 640 ms to detect energy */
> > - for (i = 0; i < 64; i++) {
> > - /* Sleep to allow link test pulses to be sent */
> > - msleep(10);
> > - rc = phy_read(phydev, MII_LAN83C185_CTRL_STATUS);
> > - if (rc < 0)
> > - return rc;
> > - if (rc & MII_LAN83C185_ENERGYON)
> > - break;
> > - }
> > + phy_read_poll_timeout(phydev, MII_LAN83C185_CTRL_STATUS,
> > + rc & MII_LAN83C185_ENERGYON, 1,
> > + 64, true);
> > + if (rc < 0)
> > + return rc;
> >
> > /* Re-enable EDPD */
> > rc = phy_read(phydev, MII_LAN83C185_CTRL_STATUS);
> > --
> > 2.25.0
> >
>
> This patch causes the kernel log to be spammed with the following when
> Ethernet cable is not connected:
> SMSC LAN8710/LAN8720 2188000.ethernet-1:00: lan87xx_read_status failed: -110
>
Kevin, I am very sorry for the trouble caused by my patch.
> It still seems to work but I think that is only a fluke.
>
> The "if (rc < 0)" is not actually checking the return value of
> phy_read_poll_timeout but is looking at the value of the register read. I
> don't think rc will ever be negative in this case. If you change the code
> to "rc = phy_read_poll_timeout(...)" so that it actually checks the error
> then the function will behave differently than before. The original code
> would only return an error if phy_read returned an error. On a timeout it
> just continued. So the "if" could be changed to "if (rc < 0 && rc !=
> -ETIMEDOUT)". But you will still get the extra messages in the log that
> were not there before.
>
Yes, My patch did change the original behavior. It will not have error message
whether it is timeout or phy_read fails, but my patch changed it and will
print some error messages. It is my mistake. I'm so sorry for that.
How do you think of the following fix?
> > /* Wait max 640 ms to detect energy */
> > - for (i = 0; i < 64; i++) {
> > - /* Sleep to allow link test pulses to be sent */
> > - msleep(10);
> > - rc = phy_read(phydev, MII_LAN83C185_CTRL_STATUS);
> > - if (rc < 0)
> > - return rc;
> > - if (rc & MII_LAN83C185_ENERGYON)
> > - break;
> > - }
> > + phy_read_poll_timeout(phydev, MII_LAN83C185_CTRL_STATUS,
> > + rc & MII_LAN83C185_ENERGYON, 1,
> > + 64, true);
> > + if (rc < 0)
> > + return rc;
ret = read_poll_timeout(phy_read, rc, rc &
MII_LAN83C185_ENERGYON || rc < 0,
1, 64, true, phydev,
MII_LAN83C185_CTRL_STATUS);
if (!ret && rc < 0)
return rc;
BR,
Dejin
>
> Kevin
[PATCH v1] PCI: controller: convert to devm_platform_ioremap_resource_byname()
Use devm_platform_ioremap_resource_byname() to simplify codes.
it contains platform_get_resource_byname() and devm_ioremap_resource().
Signed-off-by: Dejin Zheng
---
drivers/pci/controller/cadence/pcie-cadence-ep.c | 3 +--
drivers/pci/controller/cadence/pcie-cadence-host.c | 3 +--
drivers/pci/controller/pci-tegra.c | 8 +++-
drivers/pci/controller/pci-xgene.c | 3 +--
drivers/pci/controller/pcie-altera-msi.c | 3 +--
drivers/pci/controller/pcie-altera.c | 9 +++--
drivers/pci/controller/pcie-mediatek.c | 4 +---
drivers/pci/controller/pcie-rockchip.c | 5 ++---
drivers/pci/controller/pcie-xilinx-nwl.c | 7 +++
9 files changed, 16 insertions(+), 29 deletions(-)
diff --git a/drivers/pci/controller/cadence/pcie-cadence-ep.c
b/drivers/pci/controller/cadence/pcie-cadence-ep.c
index 1c15c8352125..74ffa03fde5f 100644
--- a/drivers/pci/controller/cadence/pcie-cadence-ep.c
+++ b/drivers/pci/controller/cadence/pcie-cadence-ep.c
@@ -408,8 +408,7 @@ int cdns_pcie_ep_setup(struct cdns_pcie_ep *ep)
pcie->is_rc = false;
- res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "reg");
- pcie->reg_base = devm_ioremap_resource(dev, res);
+ pcie->reg_base = devm_platform_ioremap_resource_byname(pdev, "reg");
if (IS_ERR(pcie->reg_base)) {
dev_err(dev, "missing \"reg\"\n");
return PTR_ERR(pcie->reg_base);
diff --git a/drivers/pci/controller/cadence/pcie-cadence-host.c
b/drivers/pci/controller/cadence/pcie-cadence-host.c
index 8c2543f28ba0..dcc460a54875 100644
--- a/drivers/pci/controller/cadence/pcie-cadence-host.c
+++ b/drivers/pci/controller/cadence/pcie-cadence-host.c
@@ -225,8 +225,7 @@ int cdns_pcie_host_setup(struct cdns_pcie_rc *rc)
rc->device_id = 0x;
of_property_read_u32(np, "device-id", >device_id);
- res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "reg");
- pcie->reg_base = devm_ioremap_resource(dev, res);
+ pcie->reg_base = devm_platform_ioremap_resource_byname(pdev, "reg");
if (IS_ERR(pcie->reg_base)) {
dev_err(dev, "missing \"reg\"\n");
return PTR_ERR(pcie->reg_base);
diff --git a/drivers/pci/controller/pci-tegra.c
b/drivers/pci/controller/pci-tegra.c
index e3e917243e10..3e608383df66 100644
--- a/drivers/pci/controller/pci-tegra.c
+++ b/drivers/pci/controller/pci-tegra.c
@@ -1462,7 +1462,7 @@ static int tegra_pcie_get_resources(struct tegra_pcie
*pcie)
{
struct device *dev = pcie->dev;
struct platform_device *pdev = to_platform_device(dev);
- struct resource *pads, *afi, *res;
+ struct resource *res;
const struct tegra_pcie_soc *soc = pcie->soc;
int err;
@@ -1486,15 +1486,13 @@ static int tegra_pcie_get_resources(struct tegra_pcie
*pcie)
}
}
- pads = platform_get_resource_byname(pdev, IORESOURCE_MEM, "pads");
- pcie->pads = devm_ioremap_resource(dev, pads);
+ pcie->pads = devm_platform_ioremap_resource_byname(pdev, "pads");
if (IS_ERR(pcie->pads)) {
err = PTR_ERR(pcie->pads);
goto phys_put;
}
- afi = platform_get_resource_byname(pdev, IORESOURCE_MEM, "afi");
- pcie->afi = devm_ioremap_resource(dev, afi);
+ pcie->afi = devm_platform_ioremap_resource_byname(pdev, "afi");
if (IS_ERR(pcie->afi)) {
err = PTR_ERR(pcie->afi);
goto phys_put;
diff --git a/drivers/pci/controller/pci-xgene.c
b/drivers/pci/controller/pci-xgene.c
index d1efa8ffbae1..1431a18eb02c 100644
--- a/drivers/pci/controller/pci-xgene.c
+++ b/drivers/pci/controller/pci-xgene.c
@@ -355,8 +355,7 @@ static int xgene_pcie_map_reg(struct xgene_pcie_port *port,
if (IS_ERR(port->csr_base))
return PTR_ERR(port->csr_base);
- res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "cfg");
- port->cfg_base = devm_ioremap_resource(dev, res);
+ port->cfg_base = devm_platform_ioremap_resource_byname(pdev, "cfg");
if (IS_ERR(port->cfg_base))
return PTR_ERR(port->cfg_base);
port->cfg_addr = res->start;
diff --git a/drivers/pci/controller/pcie-altera-msi.c
b/drivers/pci/controller/pcie-altera-msi.c
index 16d938920ca5..613e19af71bd 100644
--- a/drivers/pci/controller/pcie-altera-msi.c
+++ b/drivers/pci/controller/pcie-altera-msi.c
@@ -228,8 +228,7 @@ static int altera_msi_probe(struct platform_device *pdev)
mutex_init(>lock);
msi->pdev = pdev;
- res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "csr");
- msi->csr_base = devm_ioremap_resour
[tip: timers/core] drivers/clocksource/arm_arch_timer: Remove duplicate error message
The following commit has been merged into the timers/core branch of tip:
Commit-ID: d1b5e55208fd8e1c73876ab6ad1ce93485e3f5a2
Gitweb:
https://git.kernel.org/tip/d1b5e55208fd8e1c73876ab6ad1ce93485e3f5a2
Author:Dejin Zheng
AuthorDate:Wed, 29 Apr 2020 23:35:59 +08:00
Committer: Daniel Lezcano
CommitterDate: Fri, 22 May 2020 23:58:56 +02:00
drivers/clocksource/arm_arch_timer: Remove duplicate error message
The function acpi_gtdt_init() prints a message in case of
error. Remove the error message after testing if the function fails,
otherwise it is a duplicate message.
Signed-off-by: Dejin Zheng
Signed-off-by: Daniel Lezcano
Link: https://lore.kernel.org/r/20200429153559.21189-1-zhengdej...@gmail.com
---
drivers/clocksource/arm_arch_timer.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/clocksource/arm_arch_timer.c
b/drivers/clocksource/arm_arch_timer.c
index d53f4c7..befe54a 100644
--- a/drivers/clocksource/arm_arch_timer.c
+++ b/drivers/clocksource/arm_arch_timer.c
@@ -1577,10 +1577,8 @@ static int __init arch_timer_acpi_init(struct
acpi_table_header *table)
arch_timers_present |= ARCH_TIMER_TYPE_CP15;
ret = acpi_gtdt_init(table, _timer_count);
- if (ret) {
- pr_err("Failed to init GTDT table.\n");
+ if (ret)
return ret;
- }
arch_timer_ppi[ARCH_TIMER_PHYS_NONSECURE_PPI] =
acpi_gtdt_map_ppi(ARCH_TIMER_PHYS_NONSECURE_PPI);
[tip: timers/core] clocksource/drivers/arc_timer: Remove duplicate error message
The following commit has been merged into the timers/core branch of tip:
Commit-ID: 311fb70aa55174ddebb5c6022b23e58b85e9f116
Gitweb:
https://git.kernel.org/tip/311fb70aa55174ddebb5c6022b23e58b85e9f116
Author:Dejin Zheng
AuthorDate:Wed, 29 Apr 2020 23:12:23 +08:00
Committer: Daniel Lezcano
CommitterDate: Fri, 22 May 2020 23:58:56 +02:00
clocksource/drivers/arc_timer: Remove duplicate error message
The function arc_get_timer_clk() prints an error message if it fails,
remove the second error message if the function fails.
Signed-off-by: Dejin Zheng
Signed-off-by: Daniel Lezcano
Link: https://lore.kernel.org/r/20200429151223.3120-1-zhengdej...@gmail.com
---
drivers/clocksource/arc_timer.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/clocksource/arc_timer.c b/drivers/clocksource/arc_timer.c
index b29b5a7..de93dd1 100644
--- a/drivers/clocksource/arc_timer.c
+++ b/drivers/clocksource/arc_timer.c
@@ -334,10 +334,8 @@ static int __init arc_clockevent_setup(struct device_node
*node)
}
ret = arc_get_timer_clk(node);
- if (ret) {
- pr_err("clockevent: missing clk\n");
+ if (ret)
return ret;
- }
/* Needs apriori irq_set_percpu_devid() done in intc map function */
ret = request_percpu_irq(arc_timer_irq, timer_irq_handler,
[PATCH v1] PCI: dwc: convert to devm_platform_ioremap_resource_byname()
Use devm_platform_ioremap_resource_byname() to simplify codes.
it contains platform_get_resource_byname() and devm_ioremap_resource().
Signed-off-by: Dejin Zheng
---
drivers/pci/controller/dwc/pci-dra7xx.c | 11 ---
drivers/pci/controller/dwc/pci-keystone.c | 7 +++
drivers/pci/controller/dwc/pcie-artpec6.c | 12
.../pci/controller/dwc/pcie-designware-plat.c | 3 +--
drivers/pci/controller/dwc/pcie-histb.c | 7 ++-
drivers/pci/controller/dwc/pcie-intel-gw.c | 7 ++-
drivers/pci/controller/dwc/pcie-kirin.c | 17 ++---
drivers/pci/controller/dwc/pcie-qcom.c | 6 ++
drivers/pci/controller/dwc/pcie-uniphier.c | 3 +--
9 files changed, 25 insertions(+), 48 deletions(-)
diff --git a/drivers/pci/controller/dwc/pci-dra7xx.c
b/drivers/pci/controller/dwc/pci-dra7xx.c
index 6184ebc9392d..e5d0c7ac09b9 100644
--- a/drivers/pci/controller/dwc/pci-dra7xx.c
+++ b/drivers/pci/controller/dwc/pci-dra7xx.c
@@ -593,13 +593,12 @@ static int __init dra7xx_add_pcie_ep(struct dra7xx_pcie
*dra7xx,
ep = >ep;
ep->ops = _ep_ops;
- res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "ep_dbics");
- pci->dbi_base = devm_ioremap_resource(dev, res);
+ pci->dbi_base = devm_platform_ioremap_resource_byname(pdev, "ep_dbics");
if (IS_ERR(pci->dbi_base))
return PTR_ERR(pci->dbi_base);
- res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "ep_dbics2");
- pci->dbi_base2 = devm_ioremap_resource(dev, res);
+ pci->dbi_base2 =
+ devm_platform_ioremap_resource_byname(pdev, "ep_dbics2");
if (IS_ERR(pci->dbi_base2))
return PTR_ERR(pci->dbi_base2);
@@ -626,7 +625,6 @@ static int __init dra7xx_add_pcie_port(struct dra7xx_pcie
*dra7xx,
struct dw_pcie *pci = dra7xx->pci;
struct pcie_port *pp = >pp;
struct device *dev = pci->dev;
- struct resource *res;
pp->irq = platform_get_irq(pdev, 1);
if (pp->irq < 0) {
@@ -638,8 +636,7 @@ static int __init dra7xx_add_pcie_port(struct dra7xx_pcie
*dra7xx,
if (ret < 0)
return ret;
- res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "rc_dbics");
- pci->dbi_base = devm_ioremap_resource(dev, res);
+ pci->dbi_base = devm_platform_ioremap_resource_byname(pdev, "rc_dbics");
if (IS_ERR(pci->dbi_base))
return PTR_ERR(pci->dbi_base);
diff --git a/drivers/pci/controller/dwc/pci-keystone.c
b/drivers/pci/controller/dwc/pci-keystone.c
index 790679fdfa48..5ffc3b40c4f6 100644
--- a/drivers/pci/controller/dwc/pci-keystone.c
+++ b/drivers/pci/controller/dwc/pci-keystone.c
@@ -1228,8 +1228,8 @@ static int __init ks_pcie_probe(struct platform_device
*pdev)
if (!pci)
return -ENOMEM;
- res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "app");
- ks_pcie->va_app_base = devm_ioremap_resource(dev, res);
+ ks_pcie->va_app_base =
+ devm_platform_ioremap_resource_byname(pdev, "app");
if (IS_ERR(ks_pcie->va_app_base))
return PTR_ERR(ks_pcie->va_app_base);
@@ -1323,8 +1323,7 @@ static int __init ks_pcie_probe(struct platform_device
*pdev)
}
if (pci->version >= 0x480A) {
- res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "atu");
- atu_base = devm_ioremap_resource(dev, res);
+ atu_base = devm_platform_ioremap_resource_byname(pdev, "atu");
if (IS_ERR(atu_base)) {
ret = PTR_ERR(atu_base);
goto err_get_sync;
diff --git a/drivers/pci/controller/dwc/pcie-artpec6.c
b/drivers/pci/controller/dwc/pcie-artpec6.c
index 28d5a1095200..7d2cfa288b01 100644
--- a/drivers/pci/controller/dwc/pcie-artpec6.c
+++ b/drivers/pci/controller/dwc/pcie-artpec6.c
@@ -455,8 +455,7 @@ static int artpec6_add_pcie_ep(struct artpec6_pcie
*artpec6_pcie,
ep = >ep;
ep->ops = _ep_ops;
- res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "dbi2");
- pci->dbi_base2 = devm_ioremap_resource(dev, res);
+ pci->dbi_base2 = devm_platform_ioremap_resource_byname(pdev, "dbi2");
if (IS_ERR(pci->dbi_base2))
return PTR_ERR(pci->dbi_base2);
@@ -481,8 +480,6 @@ static int artpec6_pcie_probe(struct platform_device *pdev)
struct device *dev = >dev;
struct dw_pcie *pci;
struct artpec6_pcie *artpec6_pcie;
- struct resource *dbi_base;
- struct resource *phy_base;
int ret;
const struct of_device_id *match;
const struct artpec_pcie_of_data *data;
@@ -512,13 +509,12 @@ static
[PATCH v1] Documentation: devres: add missing entry for devm_platform_get_and_ioremap_resource()
The devm_platform_get_and_ioremap_resource() should be documented in devres.rst. Add the missing entry. Signed-off-by: Dejin Zheng --- Documentation/driver-api/driver-model/devres.rst | 1 + 1 file changed, 1 insertion(+) diff --git a/Documentation/driver-api/driver-model/devres.rst b/Documentation/driver-api/driver-model/devres.rst index 89681264ee2c..713b44deb0bf 100644 --- a/Documentation/driver-api/driver-model/devres.rst +++ b/Documentation/driver-api/driver-model/devres.rst @@ -314,6 +314,7 @@ IOMAP devm_platform_ioremap_resource() : calls devm_ioremap_resource() for platform device devm_platform_ioremap_resource_wc() devm_platform_ioremap_resource_byname() + devm_platform_get_and_ioremap_resource() devm_iounmap() pcim_iomap() pcim_iomap_regions() : do request_region() and iomap() on multiple BARs -- 2.25.0
