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

Groups > linux.kernel > #1265555

Re: [PATCH v2 03/11] staging: fsl-mc: Added generic MSI support for FSL-MC devices

Show all headers | View raw

On 06/11/15 23:20, Jose Rivera wrote:

[...]

>>> +static void __fsl_mc_msi_write_msg(struct fsl_mc_device *mc_bus_dev,
>>> +				   struct fsl_mc_device_irq *mc_dev_irq) {
>>> +	int error;
>>> +	struct fsl_mc_device *owner_mc_dev = mc_dev_irq->mc_dev;
>>> +	struct msi_desc *msi_desc = mc_dev_irq->msi_desc;
>>> +	struct dprc_irq_cfg irq_cfg;

[...]

>>> +	/*
>>> +	 * DPRCs and other objects use different commands to set up IRQs,
>>> +	 * so we have to differentiate them here.
>>> +	 */
>>> +	if (owner_mc_dev == mc_bus_dev) {
>>> +		/*
>>> +		 * IRQ is for the mc_bus_dev's DPRC itself
>>> +		 */
>>> +		error = dprc_set_irq(mc_bus_dev->mc_io,
>>> +				     MC_CMD_FLAG_INTR_DIS | MC_CMD_FLAG_PRI,
>>> +				     mc_bus_dev->mc_handle,
>>> +				     mc_dev_irq->dev_irq_index,
>>> +				     &irq_cfg);
>>> +		if (error < 0) {
>>> +			dev_err(&owner_mc_dev->dev,
>>> +				"dprc_set_irq() failed: %d\n", error);
>>> +		}
>>> +	} else {
>>> +		error = dprc_set_obj_irq(mc_bus_dev->mc_io,
>>> +					 MC_CMD_FLAG_INTR_DIS | MC_CMD_FLAG_PRI,
>>> +					 mc_bus_dev->mc_handle,
>>> +					 owner_mc_dev->obj_desc.type,
>>> +					 owner_mc_dev->obj_desc.id,
>>> +					 mc_dev_irq->dev_irq_index,
>>> +					 &irq_cfg);
>>> +		if (error < 0) {
>>> +			dev_err(&owner_mc_dev->dev,
>>> +				"dprc_obj_set_irq() failed: %d\n", error);
>>> +		}
>>> +	}
>>
>> It feels a bit odd that you have all of this under a single MSI umbrella,
>> and yet have to differentiate between them. Do they have a different
>> programming interface? Or is that because these dprc_set_*_irq functions
>> do some other stuff behind the scene (I'm too lazy to check...)?
>>
> Due to MC firmware API limitations, dprc_set_obj_irq can only be used
> to set IRQs for the DPRC's children not for the DPRC itself.

Right. So this makes me wonder whether or not you have the right
approach here. The logic behind the bus type was that devices with a
common programming interface would share a bus type (the odd duck being
platform which is used to implement anything else).

Your description seems to suggest that only devices behind the DPRC
share that programming interface, and that the DPRC itself is the local
weirdo.  Should it be using the platform-msi subsystem instead? Or is it
just a matter of firmware oddity?

This is probably not a big deal, but it is worth keeping it in mind,
specially if that has visible consequences (in your device tree, for
example).

Thanks,

	M.
-- 
Jazz is not dead. It just smells funny...
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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

Thread

Re: [PATCH v2 03/11] staging: fsl-mc: Added generic MSI support for  FSL-MC devices Marc Zyngier <marc.zyngier@arm.com> - 2015-11-06 19:00 +0100
  RE: [PATCH v2 03/11] staging: fsl-mc: Added generic MSI support for  FSL-MC devices Jose Rivera <German.Rivera@freescale.com> - 2015-11-07 00:30 +0100
    Re: [PATCH v2 03/11] staging: fsl-mc: Added generic MSI support for  FSL-MC devices Marc Zyngier <marc.zyngier@arm.com> - 2015-11-09 11:50 +0100
      RE: [PATCH v2 03/11] staging: fsl-mc: Added generic MSI support for  FSL-MC devices Jose Rivera <German.Rivera@freescale.com> - 2015-11-09 22:20 +0100

csiph-web