Re: [PATCH v8 4/6] pci: altera: Add Altera PCIe MSI driver

[Corentin LABBE]

> This patch adds Altera PCIe MSI driver. This soft IP supports configurable
> number of vectors, which is a dts parameter.
> 
> Signed-off-by: Ley Foon Tan <lftan@xxxxxxxxxx>
> Reviewed-by: Marc Zyngier <marc.zyngier@xxxxxxx>
> +
> +static inline void msi_writel(struct altera_msi *msi, u32 value, u32 reg)
> +{
> +	writel_relaxed(value, msi->csr_base + reg);
> +}
> +
> +static inline u32 msi_readl(struct altera_msi *msi, u32 reg)
> +{
> +	return readl_relaxed(msi->csr_base + reg);
> +}
> +

You could set value and reg parameter as const

> +
> +static int altera_irq_domain_alloc(struct irq_domain *domain, unsigned int virq,
> +				   unsigned int nr_irqs, void *args)
> +{
> +	struct altera_msi *msi = domain->host_data;
> +	unsigned long bit;
> +	u32 mask;
> +
> +	WARN_ON(nr_irqs != 1);
> +	mutex_lock(&msi->lock);
> +
> +	bit = find_first_zero_bit(msi->used, msi->num_of_vectors);
> +	if (bit >= msi->num_of_vectors)
> +		return -ENOSPC;
> +
> +	set_bit(bit, msi->used);
> +
> +	mutex_unlock(&msi->lock);
> +
> +	irq_domain_set_info(domain, virq, bit, &altera_msi_bottom_irq_chip,
> +			    domain->host_data, handle_simple_irq,
> +			    NULL, NULL);
> +
> +	mask = msi_readl(msi, MSI_INTMASK);
> +	mask |= 1 << bit;
> +	msi_writel(msi, mask, MSI_INTMASK);
> +
> +	return 0;
> +}

You do not unlock the mutex when returning -ENOSPC
And again some parameter could be set as const

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/