Patchwork [PATCHv8,21/21] ARM: dt: tegra124: add sdhci iommus bindings

login
register
mail settings
Submitter Hiroshi Doyu
Date May 30, 2014, 11:20 a.m.
Message ID <1401448834-32659-22-git-send-email-hdoyu@nvidia.com>
Download mbox | patch
Permalink /patch/354093/
State Superseded, archived
Headers show

Comments

Hiroshi Doyu - May 30, 2014, 11:20 a.m.
Add sdhci iommus bindings.

Signed-off-by: Hiroshi Doyu <hdoyu@nvidia.com>
---
 arch/arm/boot/dts/tegra124.dtsi | 4 ++++
 1 file changed, 4 insertions(+)
Stephen Warren - May 30, 2014, 4:34 p.m.
On 05/30/2014 05:20 AM, Hiroshi Doyu wrote:
> Add sdhci iommus bindings.

This isn't adding bindings, it's adding DT properties. A binding is the
documentation of the schema, not the actual data in the DT.

> diff --git a/arch/arm/boot/dts/tegra124.dtsi b/arch/arm/boot/dts/tegra124.dtsi
> index 5b339ca8a1ab..011831ffd390 100644
> --- a/arch/arm/boot/dts/tegra124.dtsi
> +++ b/arch/arm/boot/dts/tegra124.dtsi
> @@ -472,6 +472,7 @@
>  		resets = <&tegra_car 14>;
>  		reset-names = "sdhci";
>  		status = "disabled";
> +		iommus=<&smmu TEGRA_SWGROUP_CELLS(SDMMC1A)>;

Is the "iommus" property documented anywhere? I suspect not, since
there's an active ongoing discussion about how IOMMU bindings should be
structured. I suspect this series needs to wait until that discussion
completes, and the "iommus" property is documented somewhere.

There should be spaces around the = i.e. iommus = <...>;
--
To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Hiroshi Doyu - May 30, 2014, 4:44 p.m.
Stephen Warren <swarren@wwwdotorg.org> writes:

> On 05/30/2014 05:20 AM, Hiroshi Doyu wrote:
>> Add sdhci iommus bindings.
>
> This isn't adding bindings, it's adding DT properties. A binding is the
> documentation of the schema, not the actual data in the DT.
>
>> diff --git a/arch/arm/boot/dts/tegra124.dtsi b/arch/arm/boot/dts/tegra124.dtsi
>> index 5b339ca8a1ab..011831ffd390 100644
>> --- a/arch/arm/boot/dts/tegra124.dtsi
>> +++ b/arch/arm/boot/dts/tegra124.dtsi
>> @@ -472,6 +472,7 @@
>>  		resets = <&tegra_car 14>;
>>  		reset-names = "sdhci";
>>  		status = "disabled";
>> +		iommus=<&smmu TEGRA_SWGROUP_CELLS(SDMMC1A)>;
>
> Is the "iommus" property documented anywhere? I suspect not, since
> there's an active ongoing discussion about how IOMMU bindings should be
> structured. I suspect this series needs to wait until that discussion
> completes, and the "iommus" property is documented somewhere.

Yes, I'm catching up that thread now. That's why I drop document part.

If Thierry's patch(v2 or v3) is accpeted, this patch won't need any
further doc since it will use the IOMMU standard bindings("iommus = ").

--
To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Thierry Reding - June 4, 2014, 9:39 p.m.
On Fri, May 30, 2014 at 07:44:06PM +0300, Hiroshi Doyu wrote:
> 
> Stephen Warren <swarren@wwwdotorg.org> writes:
> 
> > On 05/30/2014 05:20 AM, Hiroshi Doyu wrote:
> >> Add sdhci iommus bindings.
> >
> > This isn't adding bindings, it's adding DT properties. A binding is the
> > documentation of the schema, not the actual data in the DT.
> >
> >> diff --git a/arch/arm/boot/dts/tegra124.dtsi b/arch/arm/boot/dts/tegra124.dtsi
> >> index 5b339ca8a1ab..011831ffd390 100644
> >> --- a/arch/arm/boot/dts/tegra124.dtsi
> >> +++ b/arch/arm/boot/dts/tegra124.dtsi
> >> @@ -472,6 +472,7 @@
> >>  		resets = <&tegra_car 14>;
> >>  		reset-names = "sdhci";
> >>  		status = "disabled";
> >> +		iommus=<&smmu TEGRA_SWGROUP_CELLS(SDMMC1A)>;
> >
> > Is the "iommus" property documented anywhere? I suspect not, since
> > there's an active ongoing discussion about how IOMMU bindings should be
> > structured. I suspect this series needs to wait until that discussion
> > completes, and the "iommus" property is documented somewhere.
> 
> Yes, I'm catching up that thread now. That's why I drop document part.
> 
> If Thierry's patch(v2 or v3) is accpeted, this patch won't need any
> further doc since it will use the IOMMU standard bindings("iommus = ").

The binding for the Tegra SMMU would still need to document what the
specifier means. Also I think we should change the current meaning of a
two-cell bitmask to the more explicit one cell per SWGROUP binding, that
is something like this:

	iommus = <&smmu TEGRA_SWGROUP_SDMMC1A>;

Thierry

Patch

diff --git a/arch/arm/boot/dts/tegra124.dtsi b/arch/arm/boot/dts/tegra124.dtsi
index 5b339ca8a1ab..011831ffd390 100644
--- a/arch/arm/boot/dts/tegra124.dtsi
+++ b/arch/arm/boot/dts/tegra124.dtsi
@@ -472,6 +472,7 @@ 
 		resets = <&tegra_car 14>;
 		reset-names = "sdhci";
 		status = "disabled";
+		iommus=<&smmu TEGRA_SWGROUP_CELLS(SDMMC1A)>;
 	};
 
 	sdhci@0,700b0200 {
@@ -482,6 +483,7 @@ 
 		resets = <&tegra_car 9>;
 		reset-names = "sdhci";
 		status = "disabled";
+		iommus=<&smmu TEGRA_SWGROUP_CELLS(SDMMC2A)>;
 	};
 
 	sdhci@0,700b0400 {
@@ -492,6 +494,7 @@ 
 		resets = <&tegra_car 69>;
 		reset-names = "sdhci";
 		status = "disabled";
+		iommus=<&smmu TEGRA_SWGROUP_CELLS(SDMMC3A)>;
 	};
 
 	sdhci@0,700b0600 {
@@ -502,6 +505,7 @@ 
 		resets = <&tegra_car 15>;
 		reset-names = "sdhci";
 		status = "disabled";
+		iommus=<&smmu TEGRA_SWGROUP_CELLS(SDMMC4A)>;
 	};
 
 	ahub@0,70300000 {