Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1345621

Re: [PATCH 2/2 v3] irqchip/Layerscape: Add SCFG MSI controller support

From Marc Zyngier <marc.zyngier@arm.com>
Newsgroups linux.kernel
Subject Re: [PATCH 2/2 v3] irqchip/Layerscape: Add SCFG MSI controller support
Date 2016-02-29 11:20 +0100
Message-ID <r7t5M-1ES-11@gated-at.bofh.it> (permalink)
References <r3ef0-1cV-9@gated-at.bofh.it> <r3ef0-1cV-11@gated-at.bofh.it> <r3ef0-1cV-7@gated-at.bofh.it> <r5UMO-6eo-5@gated-at.bofh.it>
Organization ARM Ltd

Show all headers | View raw


On 25/02/16 03:21, Minghuan Lian wrote:
> Hi Marc,
> 
> I am sorry for the delayed response due to the Chinese Spring Festival holiday.
> Thank you very much for the review.
> Please see my comments inline.
> 
> Thanks,
> Minghuan
> 

[...]

>>> +static int ls_scfg_msi_probe(struct platform_device *pdev) {
>>> +	struct ls_scfg_msi *msi_data;
>>> +	struct resource *res;
>>> +	int ret;
>>> +
>>> +	msi_data = devm_kzalloc(&pdev->dev, sizeof(*msi_data), GFP_KERNEL);
>>> +	if (!msi_data)
>>> +		return -ENOMEM;
>>> +
>>> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>>> +	msi_data->regs = devm_ioremap_resource(&pdev->dev, res);
>>> +	if (IS_ERR(msi_data->regs)) {
>>> +		dev_err(&pdev->dev, "failed to initialize 'regs'\n");
>>> +		return PTR_ERR(msi_data->regs);
>>> +	}
>>> +	msi_data->msiir_addr = res->start;
>>> +
>>> +	msi_data->irq = platform_get_irq(pdev, 0);
>>> +	if (msi_data->irq <= 0) {
>>> +		dev_err(&pdev->dev, "failed to get MSI irq\n");
>>> +		return -ENODEV;
>>> +	}
>>> +
>>> +	msi_data->pdev = pdev;
>>> +	msi_data->nr_irqs = MSI_MAX_IRQS;
>>
>> So this is hardcoded, always. Why do you need a nr_irqs variable at all?
> [Lian Minghuan-B31939] Currently, nr_irqs is always 32, but in the
> future, the MSI controller may be extended to support more IRQs. And,
> we may set nr_irqs the value of less than 32 to reserve some IRQs for
> special usage. So nr_irqs can bring flexibility

You have to choose: either this is configurable and you describe it in
DT, or this is not and you drop this field from the structure.

As for the "reserved interrupts", that would need to be described too.

Thanks,

	M.
-- 
Jazz is not dead. It just smells funny...

Back to linux.kernel | Previous | NextPrevious in thread | Next in thread | Find similar | Unroll thread


Thread

RE: [PATCH 2/2 v3] irqchip/Layerscape: Add SCFG MSI controller  support Minghuan Lian <minghuan.lian@nxp.com> - 2016-02-25 04:30 +0100
  Re: [PATCH 2/2 v3] irqchip/Layerscape: Add SCFG MSI controller  support Marc Zyngier <marc.zyngier@arm.com> - 2016-02-29 11:20 +0100
    RE: [PATCH 2/2 v3] irqchip/Layerscape: Add SCFG MSI controller  support Minghuan Lian <minghuan.lian@nxp.com> - 2016-03-01 12:10 +0100

csiph-web