[PATCH 0/2] amba: some cleanup

[PATCH 0/2] amba: some cleanup

[Kefeng Wang]

As the v1 of "amba: Properly handle device probe without IRQ domain"
is merged, some comments[1] from Rob and Saravana is not included,
let's send them separately again.

patch1 has been ACKed by Rob[2].
patch2 is suggested by Saravana.

[1] https://lore.kernel.org/lkml/20210816074619.177383-1-wangkefeng.wang@huawei.com/
[2] https://lore.kernel.org/linux-arm-kernel/20210827150600.78811-4-wangkefeng.wang@huawei.com/

Kefeng Wang (2):
  amba: Kill sysfs attribute file of irq
  amba: Move of_amba_device_decode_irq() into amba_probe()

 drivers/amba/bus.c | 27 ++++++---------------------
 1 file changed, 6 insertions(+), 21 deletions(-)

-- 
2.26.2

[PATCH 1/2] amba: Kill sysfs attribute file of irq

[Kefeng Wang]

As Rob said[1], there doesn't seem to be any users about the sysfs
attribute file of irq[0] and irq[1]. And we don't need to include
<asm/irq.h> as NO_IRQ has gone. Let's kill both of them.

[1] https://lkml.org/lkml/2021/8/25/461

Acked-by: Rob Herring <robh@kernel.org>
Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com>
---
 drivers/amba/bus.c | 19 ++-----------------
 1 file changed, 2 insertions(+), 17 deletions(-)

diff --git a/drivers/amba/bus.c b/drivers/amba/bus.c
index 720aa6cdd402..16d3c009505b 100644
--- a/drivers/amba/bus.c
+++ b/drivers/amba/bus.c
@@ -21,8 +21,6 @@
 #include <linux/reset.h>
 #include <linux/of_irq.h>
 
-#include <asm/irq.h>
-
 #define to_amba_driver(d)	container_of(d, struct amba_driver, drv)
 
 /* called on periphid match and class 0x9 coresight device. */
@@ -136,8 +134,6 @@ static ssize_t name##_show(struct device *_dev,				\
 static DEVICE_ATTR_RO(name)
 
 amba_attr_func(id, "%08x\n", dev->periphid);
-amba_attr_func(irq0, "%u\n", dev->irq[0]);
-amba_attr_func(irq1, "%u\n", dev->irq[1]);
 amba_attr_func(resource, "\t%016llx\t%016llx\t%016lx\n",
 	 (unsigned long long)dev->res.start, (unsigned long long)dev->res.end,
 	 dev->res.flags);
@@ -488,20 +484,9 @@ static int amba_device_try_add(struct amba_device *dev, struct resource *parent)
 
  skip_probe:
 	ret = device_add(&dev->dev);
-	if (ret)
-		goto err_release;
-
-	if (dev->irq[0])
-		ret = device_create_file(&dev->dev, &dev_attr_irq0);
-	if (ret == 0 && dev->irq[1])
-		ret = device_create_file(&dev->dev, &dev_attr_irq1);
-	if (ret == 0)
-		return ret;
-
-	device_unregister(&dev->dev);
-
  err_release:
-	release_resource(&dev->res);
+	if (ret)
+		release_resource(&dev->res);
  err_out:
 	return ret;
 
-- 
2.26.2

[PATCH 2/2] amba: Move of_amba_device_decode_irq() into amba_probe()

[Kefeng Wang]

Similar to other resources the AMBA bus "gets" for the device,
move irq obtain from amba_device_add() to amba_probe().

Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com>
---
 drivers/amba/bus.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/amba/bus.c b/drivers/amba/bus.c
index 16d3c009505b..89ecadc81f62 100644
--- a/drivers/amba/bus.c
+++ b/drivers/amba/bus.c
@@ -183,6 +183,10 @@ static int amba_probe(struct device *dev)
 	int ret;
 
 	do {
+		ret = of_amba_device_decode_irq(dev);
+		if (ret)
+			break;
+
 		ret = of_clk_set_defaults(dev->of_node, false);
 		if (ret < 0)
 			break;
@@ -396,10 +400,6 @@ static int amba_device_try_add(struct amba_device *dev, struct resource *parent)
 	void __iomem *tmp;
 	int i, ret;
 
-	ret = of_amba_device_decode_irq(dev);
-	if (ret)
-		goto err_out;
-
 	ret = request_resource(parent, &dev->res);
 	if (ret)
 		goto err_out;
-- 
2.26.2

Re: [PATCH 2/2] amba: Move of_amba_device_decode_irq() into amba_probe()

[kernel test robot]

[-- Attachment #1: Type: text/plain, Size: 8876 bytes --]

Hi Kefeng,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on linus/master]
[also build test ERROR on next-20211105]
[cannot apply to v5.15]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Kefeng-Wang/amba-some-cleanup/20211104-174611
base:   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 7ddb58cb0ecae8e8b6181d736a87667cc9ab8389
config: arm64-randconfig-r034-20211105 (attached as .config)
compiler: clang version 14.0.0 (https://github.com/llvm/llvm-project 847a6807332b13f43704327c2d30103ec0347c77)
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 arm64 cross compiling tool for clang build
        # apt-get install binutils-aarch64-linux-gnu
        # https://github.com/0day-ci/linux/commit/e7bfc31724b5810d3dade0f2b83635fec6aef601
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Kefeng-Wang/amba-some-cleanup/20211104-174611
        git checkout e7bfc31724b5810d3dade0f2b83635fec6aef601
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 ARCH=arm64 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

>> drivers/amba/bus.c:186:9: error: implicit declaration of function 'of_amba_device_decode_irq' [-Werror,-Wimplicit-function-declaration]
                   ret = of_amba_device_decode_irq(dev);
                         ^
>> drivers/amba/bus.c:375:12: error: static declaration of 'of_amba_device_decode_irq' follows non-static declaration
   static int of_amba_device_decode_irq(struct amba_device *dev)
              ^
   drivers/amba/bus.c:186:9: note: previous implicit declaration is here
                   ret = of_amba_device_decode_irq(dev);
                         ^
   2 errors generated.


vim +/of_amba_device_decode_irq +186 drivers/amba/bus.c

   173	
   174	/*
   175	 * These are the device model conversion veneers; they convert the
   176	 * device model structures to our more specific structures.
   177	 */
   178	static int amba_probe(struct device *dev)
   179	{
   180		struct amba_device *pcdev = to_amba_device(dev);
   181		struct amba_driver *pcdrv = to_amba_driver(dev->driver);
   182		const struct amba_id *id = amba_lookup(pcdrv->id_table, pcdev);
   183		int ret;
   184	
   185		do {
 > 186			ret = of_amba_device_decode_irq(dev);
   187			if (ret)
   188				break;
   189	
   190			ret = of_clk_set_defaults(dev->of_node, false);
   191			if (ret < 0)
   192				break;
   193	
   194			ret = dev_pm_domain_attach(dev, true);
   195			if (ret)
   196				break;
   197	
   198			ret = amba_get_enable_pclk(pcdev);
   199			if (ret) {
   200				dev_pm_domain_detach(dev, true);
   201				break;
   202			}
   203	
   204			pm_runtime_get_noresume(dev);
   205			pm_runtime_set_active(dev);
   206			pm_runtime_enable(dev);
   207	
   208			ret = pcdrv->probe(pcdev, id);
   209			if (ret == 0)
   210				break;
   211	
   212			pm_runtime_disable(dev);
   213			pm_runtime_set_suspended(dev);
   214			pm_runtime_put_noidle(dev);
   215	
   216			amba_put_disable_pclk(pcdev);
   217			dev_pm_domain_detach(dev, true);
   218		} while (0);
   219	
   220		return ret;
   221	}
   222	
   223	static void amba_remove(struct device *dev)
   224	{
   225		struct amba_device *pcdev = to_amba_device(dev);
   226		struct amba_driver *drv = to_amba_driver(dev->driver);
   227	
   228		pm_runtime_get_sync(dev);
   229		if (drv->remove)
   230			drv->remove(pcdev);
   231		pm_runtime_put_noidle(dev);
   232	
   233		/* Undo the runtime PM settings in amba_probe() */
   234		pm_runtime_disable(dev);
   235		pm_runtime_set_suspended(dev);
   236		pm_runtime_put_noidle(dev);
   237	
   238		amba_put_disable_pclk(pcdev);
   239		dev_pm_domain_detach(dev, true);
   240	}
   241	
   242	static void amba_shutdown(struct device *dev)
   243	{
   244		struct amba_driver *drv;
   245	
   246		if (!dev->driver)
   247			return;
   248	
   249		drv = to_amba_driver(dev->driver);
   250		if (drv->shutdown)
   251			drv->shutdown(to_amba_device(dev));
   252	}
   253	
   254	#ifdef CONFIG_PM
   255	/*
   256	 * Hooks to provide runtime PM of the pclk (bus clock).  It is safe to
   257	 * enable/disable the bus clock at runtime PM suspend/resume as this
   258	 * does not result in loss of context.
   259	 */
   260	static int amba_pm_runtime_suspend(struct device *dev)
   261	{
   262		struct amba_device *pcdev = to_amba_device(dev);
   263		int ret = pm_generic_runtime_suspend(dev);
   264	
   265		if (ret == 0 && dev->driver) {
   266			if (pm_runtime_is_irq_safe(dev))
   267				clk_disable(pcdev->pclk);
   268			else
   269				clk_disable_unprepare(pcdev->pclk);
   270		}
   271	
   272		return ret;
   273	}
   274	
   275	static int amba_pm_runtime_resume(struct device *dev)
   276	{
   277		struct amba_device *pcdev = to_amba_device(dev);
   278		int ret;
   279	
   280		if (dev->driver) {
   281			if (pm_runtime_is_irq_safe(dev))
   282				ret = clk_enable(pcdev->pclk);
   283			else
   284				ret = clk_prepare_enable(pcdev->pclk);
   285			/* Failure is probably fatal to the system, but... */
   286			if (ret)
   287				return ret;
   288		}
   289	
   290		return pm_generic_runtime_resume(dev);
   291	}
   292	#endif /* CONFIG_PM */
   293	
   294	static const struct dev_pm_ops amba_pm = {
   295		.suspend	= pm_generic_suspend,
   296		.resume		= pm_generic_resume,
   297		.freeze		= pm_generic_freeze,
   298		.thaw		= pm_generic_thaw,
   299		.poweroff	= pm_generic_poweroff,
   300		.restore	= pm_generic_restore,
   301		SET_RUNTIME_PM_OPS(
   302			amba_pm_runtime_suspend,
   303			amba_pm_runtime_resume,
   304			NULL
   305		)
   306	};
   307	
   308	/*
   309	 * Primecells are part of the Advanced Microcontroller Bus Architecture,
   310	 * so we call the bus "amba".
   311	 * DMA configuration for platform and AMBA bus is same. So here we reuse
   312	 * platform's DMA config routine.
   313	 */
   314	struct bus_type amba_bustype = {
   315		.name		= "amba",
   316		.dev_groups	= amba_dev_groups,
   317		.match		= amba_match,
   318		.uevent		= amba_uevent,
   319		.probe		= amba_probe,
   320		.remove		= amba_remove,
   321		.shutdown	= amba_shutdown,
   322		.dma_configure	= platform_dma_configure,
   323		.pm		= &amba_pm,
   324	};
   325	EXPORT_SYMBOL_GPL(amba_bustype);
   326	
   327	static int __init amba_init(void)
   328	{
   329		return bus_register(&amba_bustype);
   330	}
   331	
   332	postcore_initcall(amba_init);
   333	
   334	/**
   335	 *	amba_driver_register - register an AMBA device driver
   336	 *	@drv: amba device driver structure
   337	 *
   338	 *	Register an AMBA device driver with the Linux device model
   339	 *	core.  If devices pre-exist, the drivers probe function will
   340	 *	be called.
   341	 */
   342	int amba_driver_register(struct amba_driver *drv)
   343	{
   344		if (!drv->probe)
   345			return -EINVAL;
   346	
   347		drv->drv.bus = &amba_bustype;
   348	
   349		return driver_register(&drv->drv);
   350	}
   351	
   352	/**
   353	 *	amba_driver_unregister - remove an AMBA device driver
   354	 *	@drv: AMBA device driver structure to remove
   355	 *
   356	 *	Unregister an AMBA device driver from the Linux device
   357	 *	model.  The device model will call the drivers remove function
   358	 *	for each device the device driver is currently handling.
   359	 */
   360	void amba_driver_unregister(struct amba_driver *drv)
   361	{
   362		driver_unregister(&drv->drv);
   363	}
   364	
   365	
   366	static void amba_device_release(struct device *dev)
   367	{
   368		struct amba_device *d = to_amba_device(dev);
   369	
   370		if (d->res.parent)
   371			release_resource(&d->res);
   372		kfree(d);
   373	}
   374	
 > 375	static int of_amba_device_decode_irq(struct amba_device *dev)
   376	{
   377		struct device_node *node = dev->dev.of_node;
   378		int i, irq = 0;
   379	
   380		if (IS_ENABLED(CONFIG_OF_IRQ) && node) {
   381			/* Decode the IRQs and address ranges */
   382			for (i = 0; i < AMBA_NR_IRQS; i++) {
   383				irq = of_irq_get(node, i);
   384				if (irq < 0) {
   385					if (irq == -EPROBE_DEFER)
   386						return irq;
   387					irq = 0;
   388				}
   389	
   390				dev->irq[i] = irq;
   391			}
   392		}
   393	
   394		return 0;
   395	}
   396	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 48301 bytes --]

Re: [PATCH 2/2] amba: Move of_amba_device_decode_irq() into amba_probe()

[kernel test robot]

[-- Attachment #1: Type: text/plain, Size: 3752 bytes --]

Hi Kefeng,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on linus/master]
[also build test ERROR on next-20211105]
[cannot apply to v5.15]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Kefeng-Wang/amba-some-cleanup/20211104-174611
base:   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 7ddb58cb0ecae8e8b6181d736a87667cc9ab8389
config: arm-randconfig-r034-20211104 (attached as .config)
compiler: clang version 14.0.0 (https://github.com/llvm/llvm-project 847a6807332b13f43704327c2d30103ec0347c77)
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
        # https://github.com/0day-ci/linux/commit/e7bfc31724b5810d3dade0f2b83635fec6aef601
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Kefeng-Wang/amba-some-cleanup/20211104-174611
        git checkout e7bfc31724b5810d3dade0f2b83635fec6aef601
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 ARCH=arm 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

>> drivers/amba/bus.c:186:9: error: implicit declaration of function 'of_amba_device_decode_irq' [-Werror,-Wimplicit-function-declaration]
                   ret = of_amba_device_decode_irq(dev);
                         ^
   drivers/amba/bus.c:375:12: error: static declaration of 'of_amba_device_decode_irq' follows non-static declaration
   static int of_amba_device_decode_irq(struct amba_device *dev)
              ^
   drivers/amba/bus.c:186:9: note: previous implicit declaration is here
                   ret = of_amba_device_decode_irq(dev);
                         ^
   2 errors generated.


vim +/of_amba_device_decode_irq +186 drivers/amba/bus.c

   173	
   174	/*
   175	 * These are the device model conversion veneers; they convert the
   176	 * device model structures to our more specific structures.
   177	 */
   178	static int amba_probe(struct device *dev)
   179	{
   180		struct amba_device *pcdev = to_amba_device(dev);
   181		struct amba_driver *pcdrv = to_amba_driver(dev->driver);
   182		const struct amba_id *id = amba_lookup(pcdrv->id_table, pcdev);
   183		int ret;
   184	
   185		do {
 > 186			ret = of_amba_device_decode_irq(dev);
   187			if (ret)
   188				break;
   189	
   190			ret = of_clk_set_defaults(dev->of_node, false);
   191			if (ret < 0)
   192				break;
   193	
   194			ret = dev_pm_domain_attach(dev, true);
   195			if (ret)
   196				break;
   197	
   198			ret = amba_get_enable_pclk(pcdev);
   199			if (ret) {
   200				dev_pm_domain_detach(dev, true);
   201				break;
   202			}
   203	
   204			pm_runtime_get_noresume(dev);
   205			pm_runtime_set_active(dev);
   206			pm_runtime_enable(dev);
   207	
   208			ret = pcdrv->probe(pcdev, id);
   209			if (ret == 0)
   210				break;
   211	
   212			pm_runtime_disable(dev);
   213			pm_runtime_set_suspended(dev);
   214			pm_runtime_put_noidle(dev);
   215	
   216			amba_put_disable_pclk(pcdev);
   217			dev_pm_domain_detach(dev, true);
   218		} while (0);
   219	
   220		return ret;
   221	}
   222	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 40862 bytes --]

Re: [PATCH 2/2] amba: Move of_amba_device_decode_irq() into amba_probe()

[Kefeng Wang]

Thanks, will fix and resend after 5.16-rc1.

On 2021/11/6 0:41, kernel test robot wrote:
> Hi Kefeng,
> 
> Thank you for the patch! Yet something to improve:
> 
> [auto build test ERROR on linus/master]
> [also build test ERROR on next-20211105]
> [cannot apply to v5.15]
> [If your patch is applied to the wrong git tree, kindly drop us a note.
> And when submitting patch, we suggest to use '--base' as documented in
> https://git-scm.com/docs/git-format-patch]
> 
> url:    https://github.com/0day-ci/linux/commits/Kefeng-Wang/amba-some-cleanup/20211104-174611
> base:   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 7ddb58cb0ecae8e8b6181d736a87667cc9ab8389
> config: arm-randconfig-r034-20211104 (attached as .config)
> compiler: clang version 14.0.0 (https://github.com/llvm/llvm-project 847a6807332b13f43704327c2d30103ec0347c77)
> 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
>          # https://github.com/0day-ci/linux/commit/e7bfc31724b5810d3dade0f2b83635fec6aef601
>          git remote add linux-review https://github.com/0day-ci/linux
>          git fetch --no-tags linux-review Kefeng-Wang/amba-some-cleanup/20211104-174611
>          git checkout e7bfc31724b5810d3dade0f2b83635fec6aef601
>          # save the attached .config to linux build tree
>          COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 ARCH=arm
> 
> If you fix the issue, kindly add following tag as appropriate
> Reported-by: kernel test robot <lkp@intel.com>
> 
> All errors (new ones prefixed by >>):
> 
>>> drivers/amba/bus.c:186:9: error: implicit declaration of function 'of_amba_device_decode_irq' [-Werror,-Wimplicit-function-declaration]
>                     ret = of_amba_device_decode_irq(dev);
>                           ^
>     drivers/amba/bus.c:375:12: error: static declaration of 'of_amba_device_decode_irq' follows non-static declaration
>     static int of_amba_device_decode_irq(struct amba_device *dev)
>                ^
>     drivers/amba/bus.c:186:9: note: previous implicit declaration is here
>                     ret = of_amba_device_decode_irq(dev);
>                           ^
>     2 errors generated.
> 

Re: [PATCH 0/2] amba: some cleanup

[Russell King (Oracle)]

The ARM tree is closed; we're in the mainline merge window. Please
resend after -rc1 is released.

On Thu, Nov 04, 2021 at 05:56:41PM +0800, Kefeng Wang wrote:
> As the v1 of "amba: Properly handle device probe without IRQ domain"
> is merged, some comments[1] from Rob and Saravana is not included,
> let's send them separately again.
> 
> patch1 has been ACKed by Rob[2].
> patch2 is suggested by Saravana.
> 
> [1] https://lore.kernel.org/lkml/20210816074619.177383-1-wangkefeng.wang@huawei.com/
> [2] https://lore.kernel.org/linux-arm-kernel/20210827150600.78811-4-wangkefeng.wang@huawei.com/
> 
> Kefeng Wang (2):
>   amba: Kill sysfs attribute file of irq
>   amba: Move of_amba_device_decode_irq() into amba_probe()
> 
>  drivers/amba/bus.c | 27 ++++++---------------------
>  1 file changed, 6 insertions(+), 21 deletions(-)
> 
> -- 
> 2.26.2
> 
> 

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!