Re: [PATCH] soundwire: bus: Make IRQ handling conditionally built

[Vinod Koul]

On 18-09-23, 17:10, Charles Keepax wrote:
> SoundWire has provisions for a simple callback for the IRQ handling so
> has no hard dependency on IRQ_DOMAIN, but the recent addition of IRQ
> handling was causing builds without IRQ_DOMAIN to fail. Resolve this by
> moving the IRQ handling into its own file and only add it to the build
> when IRQ_DOMAIN is included in the kernel.
> 
> Fixes: 12a95123bfe1 ("soundwire: bus: Allow SoundWire peripherals to register IRQ handlers")
> Reported-by: kernel test robot <lkp@xxxxxxxxx>
> Closes: https://lore.kernel.org/oe-kbuild-all/202309150522.MoKeF4jx-lkp@xxxxxxxxx/
> Signed-off-by: Charles Keepax <ckeepax@xxxxxxxxxxxxxxxxxxxxx>
> ---
>  drivers/soundwire/Makefile   |  4 +++
>  drivers/soundwire/bus.c      | 31 +++----------------
>  drivers/soundwire/bus_type.c | 11 +++----
>  drivers/soundwire/irq.c      | 59 ++++++++++++++++++++++++++++++++++++
>  drivers/soundwire/irq.h      | 43 ++++++++++++++++++++++++++
>  5 files changed, 115 insertions(+), 33 deletions(-)
>  create mode 100644 drivers/soundwire/irq.c
>  create mode 100644 drivers/soundwire/irq.h
> 
> diff --git a/drivers/soundwire/Makefile b/drivers/soundwire/Makefile
> index c3d3ab3262d3a..657f5888a77b0 100644
> --- a/drivers/soundwire/Makefile
> +++ b/drivers/soundwire/Makefile
> @@ -15,6 +15,10 @@ ifdef CONFIG_DEBUG_FS
>  soundwire-bus-y += debugfs.o
>  endif
>  
> +ifdef CONFIG_IRQ_DOMAIN
> +soundwire-bus-y += irq.o
> +endif

Any reason why we cant use depends for this?


> +
>  #AMD driver
>  soundwire-amd-y :=	amd_manager.o
>  obj-$(CONFIG_SOUNDWIRE_AMD) += soundwire-amd.o
> diff --git a/drivers/soundwire/bus.c b/drivers/soundwire/bus.c
> index 1720031f35a35..0e7bc3c40f9df 100644
> --- a/drivers/soundwire/bus.c
> +++ b/drivers/soundwire/bus.c
> @@ -3,13 +3,13 @@
>  
>  #include <linux/acpi.h>
>  #include <linux/delay.h>
> -#include <linux/irq.h>
>  #include <linux/mod_devicetable.h>
>  #include <linux/pm_runtime.h>
>  #include <linux/soundwire/sdw_registers.h>
>  #include <linux/soundwire/sdw.h>
>  #include <linux/soundwire/sdw_type.h>
>  #include "bus.h"
> +#include "irq.h"
>  #include "sysfs_local.h"
>  
>  static DEFINE_IDA(sdw_bus_ida);
> @@ -25,23 +25,6 @@ static int sdw_get_id(struct sdw_bus *bus)
>  	return 0;
>  }
>  
> -static int sdw_irq_map(struct irq_domain *h, unsigned int virq,
> -		       irq_hw_number_t hw)
> -{
> -	struct sdw_bus *bus = h->host_data;
> -
> -	irq_set_chip_data(virq, bus);
> -	irq_set_chip(virq, &bus->irq_chip);
> -	irq_set_nested_thread(virq, 1);
> -	irq_set_noprobe(virq);
> -
> -	return 0;
> -}
> -
> -static const struct irq_domain_ops sdw_domain_ops = {
> -	.map	= sdw_irq_map,
> -};
> -
>  /**
>   * sdw_bus_master_add() - add a bus Master instance
>   * @bus: bus instance
> @@ -168,13 +151,9 @@ int sdw_bus_master_add(struct sdw_bus *bus, struct device *parent,
>  	bus->params.curr_bank = SDW_BANK0;
>  	bus->params.next_bank = SDW_BANK1;
>  
> -	bus->irq_chip.name = dev_name(bus->dev);
> -	bus->domain = irq_domain_create_linear(fwnode, SDW_MAX_DEVICES,
> -					       &sdw_domain_ops, bus);
> -	if (!bus->domain) {
> -		dev_err(bus->dev, "Failed to add IRQ domain\n");
> -		return -EINVAL;
> -	}
> +	ret = sdw_irq_create(bus, fwnode);
> +	if (ret)
> +		return ret;
>  
>  	return 0;
>  }
> @@ -213,7 +192,7 @@ void sdw_bus_master_delete(struct sdw_bus *bus)
>  {
>  	device_for_each_child(bus->dev, NULL, sdw_delete_slave);
>  
> -	irq_domain_remove(bus->domain);
> +	sdw_irq_delete(bus);
>  
>  	sdw_master_device_del(bus);
>  
> diff --git a/drivers/soundwire/bus_type.c b/drivers/soundwire/bus_type.c
> index fafbc284e82da..9fa93bb923d70 100644
> --- a/drivers/soundwire/bus_type.c
> +++ b/drivers/soundwire/bus_type.c
> @@ -7,6 +7,7 @@
>  #include <linux/soundwire/sdw.h>
>  #include <linux/soundwire/sdw_type.h>
>  #include "bus.h"
> +#include "irq.h"
>  #include "sysfs_local.h"
>  
>  /**
> @@ -122,11 +123,8 @@ static int sdw_drv_probe(struct device *dev)
>  	if (drv->ops && drv->ops->read_prop)
>  		drv->ops->read_prop(slave);
>  
> -	if (slave->prop.use_domain_irq) {
> -		slave->irq = irq_create_mapping(slave->bus->domain, slave->dev_num);
> -		if (!slave->irq)
> -			dev_warn(dev, "Failed to map IRQ\n");
> -	}
> +	if (slave->prop.use_domain_irq)
> +		sdw_irq_create_mapping(slave);
>  
>  	/* init the sysfs as we have properties now */
>  	ret = sdw_slave_sysfs_init(slave);
> @@ -176,8 +174,7 @@ static int sdw_drv_remove(struct device *dev)
>  	slave->probed = false;
>  
>  	if (slave->prop.use_domain_irq)
> -		irq_dispose_mapping(irq_find_mapping(slave->bus->domain,
> -						     slave->dev_num));
> +		sdw_irq_dispose_mapping(slave);
>  
>  	mutex_unlock(&slave->sdw_dev_lock);
>  
> diff --git a/drivers/soundwire/irq.c b/drivers/soundwire/irq.c
> new file mode 100644
> index 0000000000000..0c08cebb1235c
> --- /dev/null
> +++ b/drivers/soundwire/irq.c
> @@ -0,0 +1,59 @@
> +// SPDX-License-Identifier: GPL-2.0
> +// Copyright (C) 2023 Cirrus Logic, Inc. and
> +//                    Cirrus Logic International Semiconductor Ltd.
> +
> +#include <linux/device.h>
> +#include <linux/fwnode.h>
> +#include <linux/irq.h>
> +#include <linux/irqdomain.h>
> +#include <linux/soundwire/sdw.h>
> +#include "irq.h"
> +
> +static int sdw_irq_map(struct irq_domain *h, unsigned int virq,
> +		       irq_hw_number_t hw)
> +{
> +	struct sdw_bus *bus = h->host_data;
> +
> +	irq_set_chip_data(virq, bus);
> +	irq_set_chip(virq, &bus->irq_chip);
> +	irq_set_nested_thread(virq, 1);
> +	irq_set_noprobe(virq);
> +
> +	return 0;
> +}
> +
> +static const struct irq_domain_ops sdw_domain_ops = {
> +	.map	= sdw_irq_map,
> +};
> +
> +int sdw_irq_create(struct sdw_bus *bus,
> +		   struct fwnode_handle *fwnode)
> +{
> +	bus->irq_chip.name = dev_name(bus->dev);
> +
> +	bus->domain = irq_domain_create_linear(fwnode, SDW_MAX_DEVICES,
> +					       &sdw_domain_ops, bus);
> +	if (!bus->domain) {
> +		dev_err(bus->dev, "Failed to add IRQ domain\n");
> +		return -EINVAL;
> +	}
> +
> +	return 0;
> +}
> +
> +void sdw_irq_delete(struct sdw_bus *bus)
> +{
> +	irq_domain_remove(bus->domain);
> +}
> +
> +void sdw_irq_create_mapping(struct sdw_slave *slave)
> +{
> +	slave->irq = irq_create_mapping(slave->bus->domain, slave->dev_num);
> +	if (!slave->irq)
> +		dev_warn(&slave->dev, "Failed to map IRQ\n");
> +}
> +
> +void sdw_irq_dispose_mapping(struct sdw_slave *slave)
> +{
> +	irq_dispose_mapping(irq_find_mapping(slave->bus->domain, slave->dev_num));
> +}
> diff --git a/drivers/soundwire/irq.h b/drivers/soundwire/irq.h
> new file mode 100644
> index 0000000000000..b6de49419fdf5
> --- /dev/null
> +++ b/drivers/soundwire/irq.h
> @@ -0,0 +1,43 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * Copyright (C) 2023 Cirrus Logic, Inc. and
> + *                    Cirrus Logic International Semiconductor Ltd.
> + */
> +
> +#ifndef __SDW_IRQ_H
> +#define __SDW_IRQ_H
> +
> +#include <linux/soundwire/sdw.h>
> +#include <linux/fwnode.h>
> +
> +#if IS_ENABLED(CONFIG_IRQ_DOMAIN)
> +
> +int sdw_irq_create(struct sdw_bus *bus,
> +		   struct fwnode_handle *fwnode);
> +void sdw_irq_delete(struct sdw_bus *bus);
> +void sdw_irq_create_mapping(struct sdw_slave *slave);
> +void sdw_irq_dispose_mapping(struct sdw_slave *slave);
> +
> +#else // CONFIG_IRQ_DOMAIN

/* ...*/ style please

> +
> +static inline int sdw_irq_create(struct sdw_bus *bus,
> +				 struct fwnode_handle *fwnode)
> +{
> +	return 0;
> +}
> +
> +static inline void sdw_irq_delete(struct sdw_bus *bus)
> +{
> +}
> +
> +static inline void sdw_irq_create_mapping(struct sdw_slave *slave)
> +{
> +}
> +
> +static inline void sdw_irq_dispose_mapping(struct sdw_slave *slave)
> +{
> +}
> +
> +#endif // CONFIG_IRQ_DOMAIN
> +
> +#endif // __SDW_IRQ_H
> -- 
> 2.39.2

-- 
~Vinod