From: robin.murphy@arm•com (Robin Murphy)
To: linux-arm-kernel@lists•infradead.org
Subject: [RFC PATCH 1/2] fsl-mc: Added header file to provide fsl-mc bus iommu group creation
Date: Thu, 30 Jun 2016 15:14:17 +0100 [thread overview]
Message-ID: <57752939.9010504@arm.com> (raw)
In-Reply-To: <HE1PR0401MB1866C1468D3B4733B437C79BE6240@HE1PR0401MB1866.eurprd04.prod.outlook.com>
On 30/06/16 12:43, Nipun Gupta wrote:
>
>
>> -----Original Message-----
>> From: Robin Murphy [mailto:robin.murphy at arm.com]
>> Sent: Thursday, June 30, 2016 16:26
>> To: Nipun Gupta <nipun.gupta@nxp•com>; will.deacon at arm.com; Stuart Yoder
>> <stuart.yoder@nxp•com>; iommu at lists.linux-foundation.org; linux-arm-
>> kernel at lists.infradead.org
>> Cc: Bharat Bhushan <bharat.bhushan@nxp•com>
>> Subject: Re: [RFC PATCH 1/2] fsl-mc: Added header file to provide fsl-mc bus
>> iommu group creation
>>
>> On 29/06/16 18:14, Nipun Gupta wrote:
>>> Added a header file fsl_mc_smmu.h to provide basic support of creating
>>> an IOMMU group for a fsl-mc type device and also provide helper macro
>>> to get the stream ID of fsl-mc tyoe device.
>>>
>>> Signed-off-by: Nipun Gupta <nipun.gupta@nxp•com>
>>> Signed-off-by: Bharat Bhushan <bharat.bhushan@nxp•com>
>>> ---
>>> drivers/staging/fsl-mc/include/fsl_mc_smmu.h | 45
>>> ++++++++++++++++++++++++++++
>>> 1 file changed, 45 insertions(+)
>>> create mode 100644 drivers/staging/fsl-mc/include/fsl_mc_smmu.h
>>>
>>> diff --git a/drivers/staging/fsl-mc/include/fsl_mc_smmu.h
>>> b/drivers/staging/fsl-mc/include/fsl_mc_smmu.h
>>> new file mode 100644
>>> index 0000000..9dff5ba
>>> --- /dev/null
>>> +++ b/drivers/staging/fsl-mc/include/fsl_mc_smmu.h
>>> @@ -0,0 +1,45 @@
>>> +/*
>>> + * Copyright (C) 2016 Freescale Semiconductor, Inc.
>>> + * Author: Nipun Gupta <nipun.gupta@freescale•com>
>>> + *
>>> + * This file is licensed under the terms of the GNU General Public
>>> + * License version 2. This program is licensed "as is" without any
>>> + * warranty of any kind, whether express or implied.
>>> + */
>>> +
>>> +#ifndef _FSL_MC_SMMU_H_
>>> +#define _FSL_MC_SMMU_H_
>>> +
>>> +#include <../drivers/staging/fsl-mc/include/mc.h>
>>> +
>>> +/* Macro to get the MC device ICID (Stream ID) */ #define
>>> +fslmc_dev_streamid(_dev) (to_fsl_mc_device(_dev)->icid)
>>
>> Is the the full 15-bit ICID from the MC's point of view, just the 7 bits that are
>> actually routed to the SMMU, or the actual stream ID seen by the SMMU? None
>> of those three are necessarily the same, and unless it's the third then I don't see
>> the point of patches adding incomplete code which isn't going to work.
>
> Hi Robin,
>
> It's the second one where just 7 bits are maintained in the 'fsl_mc_device--->icid' which is programmed in SMMU. Right, once the SMR masking support is there then only these patches would make more sense.
> We have sent these patches to get some early comments and to have you guys well informed beforehand about the changes which we require in the SMMU driver specific to fsl-mc bus :).
Indeed, it's just kinda hard to review incomplete code, especially when
it's the parts which don't exist yet which are the most significant ;)
Either way, I'm deep in the middle of reworking SMMUv2 generic
bindings[1] based on the latest iteration of the core code and
iommu_fwspec[2], which removes the need for nearly all the bus-specific
business within the driver (it's getting too big to be 4.8 material now,
but I'll have something to post shortly). In that context, if the fsl-mc
driver could process an "iommu-map" property on the MC node to plumb the
ICID-to-stream-ID relationship into of_xlate, and inherit bus->iommu_ops
from its parent bus, there shouldn't be any need to directly touch the
SMMU driver at all.
Robin.
[1]:http://thread.gmane.org/gmane.linux.kernel.iommu/12454
[2]:http://thread.gmane.org/gmane.linux.kernel.iommu/14303
>
> Thanks,
> Nipun
>
>>
>>> +/* Macro to get the container device of a MC device */ #define
>>> +fslmc_cont_dev(_dev) ((to_fsl_mc_device(dev)->flags & \
>>> + FSL_MC_IS_DPRC) ? (_dev) : (_dev->parent))
>>> +
>>> +/* Macro to check if a device is a container device */ #define
>>> +is_cont_dev(_dev) (to_fsl_mc_device(_dev)->flags & FSL_MC_IS_DPRC)
>>> +
>>> +static struct iommu_group *fslmc_device_group(struct device *dev) {
>>> + struct device *cont_dev = fslmc_cont_dev(dev);
>>> + struct iommu_group *group;
>>> +
>>> + /* Container device is responsible for creating the iommu group */
>>> + if (is_cont_dev(dev)) {
>>> + group = iommu_group_alloc();
>>> +
>>> + if (IS_ERR(group))
>>> + return NULL;
>>> + } else {
>>> + get_device(cont_dev);
>>> + group = iommu_group_get(cont_dev);
>>> + put_device(cont_dev);
>>> + }
>>> +
>>> + return group;
>>> +}
>>
>> In isolation, though, this part seems perfectly reasonable.
>>
>> Robin.
>>
>>> +
>>> +#endif /* _FSL_MC_SMMU_H_ */
>>>
>
next prev parent reply other threads:[~2016-06-30 14:14 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-06-29 17:14 [RFC PATCH 1/2] fsl-mc: Added header file to provide fsl-mc bus iommu group creation Nipun Gupta
2016-06-29 17:14 ` [RFC PATCH 2/2] iommu/arm-smmu: Add support for the fsl-mc bus Nipun Gupta
2016-06-30 11:23 ` Robin Murphy
2016-06-30 12:11 ` Nipun Gupta
2016-06-30 10:55 ` [RFC PATCH 1/2] fsl-mc: Added header file to provide fsl-mc bus iommu group creation Robin Murphy
2016-06-30 11:43 ` Nipun Gupta
2016-06-30 14:14 ` Robin Murphy [this message]
2016-07-01 15:11 ` Nipun Gupta
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=57752939.9010504@arm.com \
--to=robin.murphy@arm$(echo .)com \
--cc=linux-arm-kernel@lists$(echo .)infradead.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox