From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754609AbdEQO4U convert rfc822-to-8bit (ORCPT ); Wed, 17 May 2017 10:56:20 -0400 Received: from us-smtp-delivery-212.mimecast.com ([216.205.24.212]:35992 "EHLO us-smtp-delivery-212.mimecast.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751607AbdEQO4S (ORCPT ); Wed, 17 May 2017 10:56:18 -0400 Subject: Re: [PATCH v4 1/2] PCI: Add tango MSI controller support From: Marc Gonzalez To: Marc Zyngier , Thomas Gleixner , Bjorn Helgaas , Jiang Liu CC: Robin Murphy , Lorenzo Pieralisi , Liviu Dudau , David Laight , linux-pci , Linux ARM , Thibaud Cornic , Phuong Nguyen , LKML , Mason References: <1f6b50c7-4887-838e-8c7d-c014d82b6d8e@sigmadesigns.com> Message-ID: <8e56647d-3b2d-9bfc-b59e-79a2efaa7826@sigmadesigns.com> Date: Wed, 17 May 2017 16:56:08 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Firefox/52.0 SeaMonkey/2.49 MIME-Version: 1.0 In-Reply-To: <1f6b50c7-4887-838e-8c7d-c014d82b6d8e@sigmadesigns.com> X-Originating-IP: [172.27.0.114] X-MC-Unique: f1rLUxj_Mkm2gAPvyKjr4w-1 Content-Type: text/plain; charset=ISO-8859-15 Content-Transfer-Encoding: 8BIT Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 20/04/2017 16:28, Marc Gonzalez wrote: > +static int tango_set_affinity(struct irq_data *data, > + const struct cpumask *mask, bool force) > +{ > + return -EINVAL; > +} > + > +static struct irq_chip tango_chip = { > + .irq_ack = tango_ack, > + .irq_mask = tango_mask, > + .irq_unmask = tango_unmask, > + .irq_set_affinity = tango_set_affinity, > + .irq_compose_msi_msg = tango_compose_msi_msg, > +}; Hmmm... I'm wondering why .irq_set_affinity is required. static int setup_affinity(struct irq_desc *desc, struct cpumask *mask) first calls __irq_can_set_affinity() to check whether desc->irq_data.chip->irq_set_affinity) exists. then calls irq_do_set_affinity(&desc->irq_data, mask, false); which calls chip->irq_set_affinity(data, mask, force); = msi_domain_set_affinity() which calls parent->chip->irq_set_affinity() unconditionally. Would it make sense to test that the callback is implemented before calling it? [ 0.723895] Unable to handle kernel NULL pointer dereference at virtual address 00000000 ... [ 1.135809] [] (msi_domain_set_affinity) from [] (irq_do_set_affinity+0x18/0x48) [ 1.144990] [] (irq_do_set_affinity) from [] (setup_affinity+0x84/0xd4) [ 1.153384] [] (setup_affinity) from [] (__setup_irq+0x40c/0x5d4) [ 1.161254] [] (__setup_irq) from [] (request_threaded_irq+0xe4/0x184) [ 1.169569] [] (request_threaded_irq) from [] (aer_probe+0x9c/0x218) [ 1.177704] [] (aer_probe) from [] (pcie_port_probe_service+0x34/0x70) [ 1.186017] [] (pcie_port_probe_service) from [] (really_probe+0x1c4/0x250) [ 1.194763] [] (really_probe) from [] (__device_attach_driver+0xa4/0xe0) [ 1.203245] [] (__device_attach_driver) from [] (bus_for_each_drv+0x60/0x94) [ 1.212076] [] (bus_for_each_drv) from [] (__device_attach+0x9c/0xdc) [ 1.220296] [] (__device_attach) from [] (device_initial_probe+0xc/0x10) [ 1.228777] [] (device_initial_probe) from [] (bus_probe_device+0x84/0x8c) [ 1.237433] [] (bus_probe_device) from [] (device_add+0x3cc/0x548) [ 1.245390] [] (device_add) from [] (device_register+0x14/0x18) [ 1.253086] [] (device_register) from [] (pcie_port_device_register+0x3b0/0x450) [ 1.262267] [] (pcie_port_device_register) from [] (pcie_portdrv_probe+0x2c/0x50) [ 1.271539] [] (pcie_portdrv_probe) from [] (pci_device_probe+0x7c/0xc8) [ 1.280022] [] (pci_device_probe) from [] (really_probe+0x178/0x250) [ 1.288154] [] (really_probe) from [] (__device_attach_driver+0xa4/0xe0) [ 1.296635] [] (__device_attach_driver) from [] (bus_for_each_drv+0x60/0x94) [ 1.305465] [] (bus_for_each_drv) from [] (__device_attach+0x9c/0xdc) [ 1.313684] [] (__device_attach) from [] (device_attach+0xc/0x10) [ 1.321559] [] (device_attach) from [] (pci_bus_add_device+0x44/0x90) [ 1.329778] [] (pci_bus_add_device) from [] (pci_bus_add_devices+0x3c/0x80) [ 1.338523] [] (pci_bus_add_devices) from [] (pci_host_common_probe+0x100/0x314) [ 1.347703] [] (pci_host_common_probe) from [] (tango_pcie_probe+0x138/0x340) [ 1.356624] [] (tango_pcie_probe) from [] (platform_drv_probe+0x34/0x6c)