On Mon, Mar 03 2025 at 19:16, Inochi Amaoto wrote:
> Add support for Sophgo SG2044 MSI interrupt controller.
This patch fails to apply on top of:
git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git irq/drivers
Please always ensure that your patches apply against the tree/branch
into which they are supposed to be merged. Grabbing random patches from
the mailing list as base is not sufficient. It's clearly documented
against what you should work.
> +struct sg2042_msi_of_data {
There is nothing specific to OF in this data structure. This structure
contains the chip and the MSI parent ops of each variant. So something
like sg204x_chip_info is way more descriptive.
> + const struct irq_chip *irqchip;
> + const struct msi_parent_ops *parent_ops;
> +};
> +
> struct sg2042_msi_chipdata {
and rename that one to sg204x_... as it is not longer sg2042 specific.
> void __iomem *reg_clr; // clear reg, see TRM, 10.1.33, GP_INTR0_CLR
>
> @@ -29,8 +34,10 @@ struct sg2042_msi_chipdata {
> u32 irq_first; // The vector number that MSIs starts
> u32 num_irqs; // The number of vectors for MSIs
>
> - DECLARE_BITMAP(msi_map, SG2042_MAX_MSI_VECTOR);
> + unsigned long *msi_map;
> struct mutex msi_map_lock; // lock for msi_map
> +
> + const struct sg2042_msi_of_data *data;
Please keep the tabular formatting of this struct. See:
https://www.kernel.org/doc/html/latest/process/maintainer-tip.html#coding-style-notes
> };
>
> static int sg2042_msi_allocate_hwirq(struct sg2042_msi_chipdata *data, int num_req)
> @@ -81,6 +88,37 @@ static const struct irq_chip sg2042_msi_middle_irq_chip = {
> .irq_compose_msi_msg = sg2042_msi_irq_compose_msi_msg,
> };
>
> +static void sg2044_msi_irq_ack(struct irq_data *d)
> +{
> + struct sg2042_msi_chipdata *data = irq_data_get_irq_chip_data(d);
> +
> + writel(0, (unsigned int *)data->reg_clr + d->hwirq);
> +
Pointless newline
> + irq_chip_ack_parent(d);
> +}
> +
> +static void sg2044_msi_irq_compose_msi_msg(struct irq_data *d,
> + struct msi_msg *msg)
No line break required. Please use up to 100 characters.
> static int sg2042_msi_parent_domain_alloc(struct irq_domain *domain,
> unsigned int virq, int hwirq)
> {
> @@ -119,7 +157,7 @@ static int sg2042_msi_middle_domain_alloc(struct irq_domain *domain,
> goto err_hwirq;
>
> irq_domain_set_hwirq_and_chip(domain, virq + i, hwirq + i,
> - &sg2042_msi_middle_irq_chip, data);
> + data->data->irqchip, data);
The conversion of the existing code to this should be a preparatory patch
for ease of review and the support for the new chip built on top.
Also please come up with a sensible name for this new 'data' pointer.
data->data->
is horribly unintuitive. It's not the same data type.
data->chip_info
or such makes it clear what this is about.
Thanks,
tglx
