To simplify the code, alloc necessary data at once.
Signed-off-by: Hiroshi DOYU <[email protected]>
---
This patch requires:
[PATCH 1/2] iommu/tegra: smmu: Add device tree support for SMMU
http://marc.info/?l=linux-tegra&m=133663641107327&w=2
Also the above patch requires:
[PATCH 1/1] dt: Add general DMA window parser
http://marc.info/?l=linux-tegra&m=133671302703840&w=2
---
drivers/iommu/tegra-smmu.c | 29 +++++++++--------------------
1 files changed, 9 insertions(+), 20 deletions(-)
diff --git a/drivers/iommu/tegra-smmu.c b/drivers/iommu/tegra-smmu.c
index dbaba77..e4acd44 100644
--- a/drivers/iommu/tegra-smmu.c
+++ b/drivers/iommu/tegra-smmu.c
@@ -241,8 +241,6 @@ struct smmu_device {
spinlock_t lock;
char *name;
struct device *dev;
- int num_as;
- struct smmu_as *as; /* Run-time allocated array */
struct page *avp_vector_page; /* dummy page shared by all AS's */
/*
@@ -254,6 +252,9 @@ struct smmu_device {
unsigned long asid_security;
struct device_node *ahb;
+
+ int num_as;
+ struct smmu_as as[0]; /* Run-time allocated array */
};
static struct smmu_device *smmu_handle; /* unique for a system */
@@ -902,15 +903,18 @@ static int tegra_smmu_probe(struct platform_device *pdev)
struct device *dev = &pdev->dev;
int i, asids, err = 0;
dma_addr_t base;
- size_t size;
- const void *prop;
+ size_t bytes, size;
if (smmu_handle)
return -EIO;
BUILD_BUG_ON(PAGE_SHIFT != SMMU_PAGE_SHIFT);
- smmu = devm_kzalloc(dev, sizeof(*smmu), GFP_KERNEL);
+ if (of_property_read_u32(dev->of_node, "nvidia,#asids", &asids))
+ return -ENODEV;
+
+ bytes = sizeof(*smmu) + asids * sizeof(*smmu->as);
+ smmu = devm_kzalloc(dev, bytes, GFP_KERNEL);
if (!smmu) {
dev_err(dev, "failed to allocate smmu_device\n");
return -ENOMEM;
@@ -939,13 +943,6 @@ static int tegra_smmu_probe(struct platform_device *pdev)
if (!size)
return -EINVAL;
- prop = of_get_property(dev->of_node, "nvidia,#asids", NULL);
- if (!prop)
- return -ENODEV;
- asids = be32_to_cpup(prop);
- if (!asids)
- return -ENODEV;
-
smmu->ahb = of_parse_phandle(dev->of_node, "nvidia,ahb", 0);
if (!smmu->ahb)
return -ENODEV;
@@ -960,14 +957,6 @@ static int tegra_smmu_probe(struct platform_device *pdev)
smmu->translation_enable_2 = ~0;
smmu->asid_security = 0;
- smmu->as = devm_kzalloc(dev,
- sizeof(smmu->as[0]) * smmu->num_as, GFP_KERNEL);
- if (!smmu->as) {
- dev_err(dev, "failed to allocate smmu_as\n");
- err = -ENOMEM;
- goto fail;
- }
-
for (i = 0; i < smmu->num_as; i++) {
struct smmu_as *as = &smmu->as[i];
--
1.7.5.4
Remove unnecessary cleanup procedures with devm_*() functions.
Signed-off-by: Hiroshi DOYU <[email protected]>
---
drivers/iommu/tegra-smmu.c | 37 ++++++-------------------------------
1 files changed, 6 insertions(+), 31 deletions(-)
diff --git a/drivers/iommu/tegra-smmu.c b/drivers/iommu/tegra-smmu.c
index e4acd44..9d0cc33 100644
--- a/drivers/iommu/tegra-smmu.c
+++ b/drivers/iommu/tegra-smmu.c
@@ -972,51 +972,26 @@ static int tegra_smmu_probe(struct platform_device *pdev)
spin_lock_init(&smmu->lock);
err = smmu_setup_regs(smmu);
if (err)
- goto fail;
+ return err;
platform_set_drvdata(pdev, smmu);
smmu->avp_vector_page = alloc_page(GFP_KERNEL);
if (!smmu->avp_vector_page)
- goto fail;
+ return -ENOMEM;
smmu_handle = smmu;
return 0;
-
-fail:
- if (smmu->avp_vector_page)
- __free_page(smmu->avp_vector_page);
- if (smmu && smmu->as) {
- for (i = 0; i < smmu->num_as; i++) {
- if (smmu->as[i].pdir_page) {
- ClearPageReserved(smmu->as[i].pdir_page);
- __free_page(smmu->as[i].pdir_page);
- }
- }
- devm_kfree(dev, smmu->as);
- }
- devm_kfree(dev, smmu);
- return err;
}
static int tegra_smmu_remove(struct platform_device *pdev)
{
struct smmu_device *smmu = platform_get_drvdata(pdev);
- struct device *dev = smmu->dev;
+ int i;
smmu_write(smmu, SMMU_CONFIG_DISABLE, SMMU_CONFIG);
- platform_set_drvdata(pdev, NULL);
- if (smmu->as) {
- int i;
-
- for (i = 0; i < smmu->num_as; i++)
- free_pdir(&smmu->as[i]);
- devm_kfree(dev, smmu->as);
- }
- if (smmu->avp_vector_page)
- __free_page(smmu->avp_vector_page);
- if (smmu->regs)
- devm_iounmap(dev, smmu->regs);
- devm_kfree(dev, smmu);
+ for (i = 0; i < smmu->num_as; i++)
+ free_pdir(&smmu->as[i]);
+ __free_page(smmu->avp_vector_page);
smmu_handle = NULL;
return 0;
}
--
1.7.5.4
On 05/14/2012 01:16 PM, Hiroshi DOYU wrote:
> To simplify the code, alloc necessary data at once.
>
> Signed-off-by: Hiroshi DOYU <[email protected]>
> ---
> This patch requires:
>
> [PATCH 1/2] iommu/tegra: smmu: Add device tree support for SMMU
> http://marc.info/?l=linux-tegra&m=133663641107327&w=2
>
> Also the above patch requires:
>
> [PATCH 1/1] dt: Add general DMA window parser
> http://marc.info/?l=linux-tegra&m=133671302703840&w=2
I know I've been harping on about dependencies, but you typically only
need to mention them if the dependencies are not already checked into
the branch you expect this patch to be checked into.
> - smmu = devm_kzalloc(dev, sizeof(*smmu), GFP_KERNEL);
> + if (of_property_read_u32(dev->of_node, "nvidia,#asids", &asids))
> + return -ENODEV;
I believe you need to change the asids variable from int to u32 to avoid
a warning here.
Aside from that, the series,
Acked-by: Stephen Warren <[email protected]>
Stephen Warren <[email protected]> wrote @ Tue, 15 May 2012 01:34:15 +0200:
> On 05/14/2012 01:16 PM, Hiroshi DOYU wrote:
> > To simplify the code, alloc necessary data at once.
> >
> > Signed-off-by: Hiroshi DOYU <[email protected]>
> > ---
> > This patch requires:
> >
> > [PATCH 1/2] iommu/tegra: smmu: Add device tree support for SMMU
> > http://marc.info/?l=linux-tegra&m=133663641107327&w=2
> >
> > Also the above patch requires:
> >
> > [PATCH 1/1] dt: Add general DMA window parser
> > http://marc.info/?l=linux-tegra&m=133671302703840&w=2
>
> I know I've been harping on about dependencies, but you typically only
> need to mention them if the dependencies are not already checked into
> the branch you expect this patch to be checked into.
There was a bit time gap. The former one is merged now. The latter
dt/DMA window patch isn't checked in.
To simplify the code, alloc necessary data at once.
Signed-off-by: Hiroshi DOYU <[email protected]>
Acked-by: Stephen Warren <[email protected]>
---
This patch requires:
[PATCH 1/1] dt: Add general DMA window parser
http://marc.info/?l=linux-tegra&m=133671302703840&w=2
---
drivers/iommu/tegra-smmu.c | 32 +++++++++++---------------------
1 files changed, 11 insertions(+), 21 deletions(-)
diff --git a/drivers/iommu/tegra-smmu.c b/drivers/iommu/tegra-smmu.c
index dbaba77..5eb32b3 100644
--- a/drivers/iommu/tegra-smmu.c
+++ b/drivers/iommu/tegra-smmu.c
@@ -241,8 +241,6 @@ struct smmu_device {
spinlock_t lock;
char *name;
struct device *dev;
- int num_as;
- struct smmu_as *as; /* Run-time allocated array */
struct page *avp_vector_page; /* dummy page shared by all AS's */
/*
@@ -254,6 +252,9 @@ struct smmu_device {
unsigned long asid_security;
struct device_node *ahb;
+
+ int num_as;
+ struct smmu_as as[0]; /* Run-time allocated array */
};
static struct smmu_device *smmu_handle; /* unique for a system */
@@ -900,17 +901,21 @@ static int tegra_smmu_probe(struct platform_device *pdev)
{
struct smmu_device *smmu;
struct device *dev = &pdev->dev;
- int i, asids, err = 0;
+ int i, err = 0;
+ u32 asids;
dma_addr_t base;
- size_t size;
- const void *prop;
+ size_t bytes, size;
if (smmu_handle)
return -EIO;
BUILD_BUG_ON(PAGE_SHIFT != SMMU_PAGE_SHIFT);
- smmu = devm_kzalloc(dev, sizeof(*smmu), GFP_KERNEL);
+ if (of_property_read_u32(dev->of_node, "nvidia,#asids", &asids))
+ return -ENODEV;
+
+ bytes = sizeof(*smmu) + asids * sizeof(*smmu->as);
+ smmu = devm_kzalloc(dev, bytes, GFP_KERNEL);
if (!smmu) {
dev_err(dev, "failed to allocate smmu_device\n");
return -ENOMEM;
@@ -939,13 +944,6 @@ static int tegra_smmu_probe(struct platform_device *pdev)
if (!size)
return -EINVAL;
- prop = of_get_property(dev->of_node, "nvidia,#asids", NULL);
- if (!prop)
- return -ENODEV;
- asids = be32_to_cpup(prop);
- if (!asids)
- return -ENODEV;
-
smmu->ahb = of_parse_phandle(dev->of_node, "nvidia,ahb", 0);
if (!smmu->ahb)
return -ENODEV;
@@ -960,14 +958,6 @@ static int tegra_smmu_probe(struct platform_device *pdev)
smmu->translation_enable_2 = ~0;
smmu->asid_security = 0;
- smmu->as = devm_kzalloc(dev,
- sizeof(smmu->as[0]) * smmu->num_as, GFP_KERNEL);
- if (!smmu->as) {
- dev_err(dev, "failed to allocate smmu_as\n");
- err = -ENOMEM;
- goto fail;
- }
-
for (i = 0; i < smmu->num_as; i++) {
struct smmu_as *as = &smmu->as[i];
--
1.7.5.4
Remove unnecessary cleanup procedures with devm_*() functions.
Signed-off-by: Hiroshi DOYU <[email protected]>
Acked-by: Stephen Warren <[email protected]>
---
drivers/iommu/tegra-smmu.c | 37 ++++++-------------------------------
1 files changed, 6 insertions(+), 31 deletions(-)
diff --git a/drivers/iommu/tegra-smmu.c b/drivers/iommu/tegra-smmu.c
index 5eb32b3..9c2a99e 100644
--- a/drivers/iommu/tegra-smmu.c
+++ b/drivers/iommu/tegra-smmu.c
@@ -973,51 +973,26 @@ static int tegra_smmu_probe(struct platform_device *pdev)
spin_lock_init(&smmu->lock);
err = smmu_setup_regs(smmu);
if (err)
- goto fail;
+ return err;
platform_set_drvdata(pdev, smmu);
smmu->avp_vector_page = alloc_page(GFP_KERNEL);
if (!smmu->avp_vector_page)
- goto fail;
+ return -ENOMEM;
smmu_handle = smmu;
return 0;
-
-fail:
- if (smmu->avp_vector_page)
- __free_page(smmu->avp_vector_page);
- if (smmu && smmu->as) {
- for (i = 0; i < smmu->num_as; i++) {
- if (smmu->as[i].pdir_page) {
- ClearPageReserved(smmu->as[i].pdir_page);
- __free_page(smmu->as[i].pdir_page);
- }
- }
- devm_kfree(dev, smmu->as);
- }
- devm_kfree(dev, smmu);
- return err;
}
static int tegra_smmu_remove(struct platform_device *pdev)
{
struct smmu_device *smmu = platform_get_drvdata(pdev);
- struct device *dev = smmu->dev;
+ int i;
smmu_write(smmu, SMMU_CONFIG_DISABLE, SMMU_CONFIG);
- platform_set_drvdata(pdev, NULL);
- if (smmu->as) {
- int i;
-
- for (i = 0; i < smmu->num_as; i++)
- free_pdir(&smmu->as[i]);
- devm_kfree(dev, smmu->as);
- }
- if (smmu->avp_vector_page)
- __free_page(smmu->avp_vector_page);
- if (smmu->regs)
- devm_iounmap(dev, smmu->regs);
- devm_kfree(dev, smmu);
+ for (i = 0; i < smmu->num_as; i++)
+ free_pdir(&smmu->as[i]);
+ __free_page(smmu->avp_vector_page);
smmu_handle = NULL;
return 0;
}
--
1.7.5.4
Stephen Warren <[email protected]> wrote @ Tue, 15 May 2012 01:34:15 +0200:
> On 05/14/2012 01:16 PM, Hiroshi DOYU wrote:
> > To simplify the code, alloc necessary data at once.
> >
> > Signed-off-by: Hiroshi DOYU <[email protected]>
> > ---
> > This patch requires:
> >
> > [PATCH 1/2] iommu/tegra: smmu: Add device tree support for SMMU
> > http://marc.info/?l=linux-tegra&m=133663641107327&w=2
> >
> > Also the above patch requires:
> >
> > [PATCH 1/1] dt: Add general DMA window parser
> > http://marc.info/?l=linux-tegra&m=133671302703840&w=2
>
> I know I've been harping on about dependencies, but you typically only
> need to mention them if the dependencies are not already checked into
> the branch you expect this patch to be checked into.
>
> > - smmu = devm_kzalloc(dev, sizeof(*smmu), GFP_KERNEL);
> > + if (of_property_read_u32(dev->of_node, "nvidia,#asids", &asids))
> > + return -ENODEV;
>
> I believe you need to change the asids variable from int to u32 to avoid
> a warning here.
There's no warning but it's allowed because of "-Wno-pointer-sign". It
seems that this flag has been used for long in kernel, and there are
many places. Is this conversion allowed because it's expeced that the
author should know the range of value, or any other reason?
*1:
-Wno-pointer-sign
Don't warn for pointer argument passing or assignment with
different signedness. Only useful in the negative form since this
warning is enabled by default. This option is only supported for C
and Objective-C.
On 05/15/2012 02:26 AM, Hiroshi Doyu wrote:
> Stephen Warren <[email protected]> wrote @ Tue, 15 May 2012 01:34:15 +0200:
>
>> On 05/14/2012 01:16 PM, Hiroshi DOYU wrote:
>>> To simplify the code, alloc necessary data at once.
>>>
>>> Signed-off-by: Hiroshi DOYU <[email protected]>
>>> ---
>>> This patch requires:
>>>
>>> [PATCH 1/2] iommu/tegra: smmu: Add device tree support for SMMU
>>> http://marc.info/?l=linux-tegra&m=133663641107327&w=2
>>>
>>> Also the above patch requires:
>>>
>>> [PATCH 1/1] dt: Add general DMA window parser
>>> http://marc.info/?l=linux-tegra&m=133671302703840&w=2
>>
>> I know I've been harping on about dependencies, but you typically only
>> need to mention them if the dependencies are not already checked into
>> the branch you expect this patch to be checked into.
>>
>>> - smmu = devm_kzalloc(dev, sizeof(*smmu), GFP_KERNEL);
>>> + if (of_property_read_u32(dev->of_node, "nvidia,#asids", &asids))
>>> + return -ENODEV;
>>
>> I believe you need to change the asids variable from int to u32 to avoid
>> a warning here.
>
> There's no warning but it's allowed because of "-Wno-pointer-sign". It
That's odd. I'm sure I have seen this warning recently when calling this
API, but you're right, I'm not able to trigger that warning right now,
so this is fine.
Stephen Warren <[email protected]> wrote @ Tue, 15 May 2012 18:09:13 +0200:
> On 05/15/2012 02:26 AM, Hiroshi Doyu wrote:
> > Stephen Warren <[email protected]> wrote @ Tue, 15 May 2012 01:34:15 +0200:
> >
> >> On 05/14/2012 01:16 PM, Hiroshi DOYU wrote:
> >>> To simplify the code, alloc necessary data at once.
> >>>
> >>> Signed-off-by: Hiroshi DOYU <[email protected]>
> >>> ---
> >>> This patch requires:
> >>>
> >>> [PATCH 1/2] iommu/tegra: smmu: Add device tree support for SMMU
> >>> http://marc.info/?l=linux-tegra&m=133663641107327&w=2
> >>>
> >>> Also the above patch requires:
> >>>
> >>> [PATCH 1/1] dt: Add general DMA window parser
> >>> http://marc.info/?l=linux-tegra&m=133671302703840&w=2
> >>
> >> I know I've been harping on about dependencies, but you typically only
> >> need to mention them if the dependencies are not already checked into
> >> the branch you expect this patch to be checked into.
> >>
> >>> - smmu = devm_kzalloc(dev, sizeof(*smmu), GFP_KERNEL);
> >>> + if (of_property_read_u32(dev->of_node, "nvidia,#asids", &asids))
> >>> + return -ENODEV;
> >>
> >> I believe you need to change the asids variable from int to u32 to avoid
> >> a warning here.
> >
> > There's no warning but it's allowed because of "-Wno-pointer-sign". It
>
> That's odd. I'm sure I have seen this warning recently when calling this
> API, but you're right, I'm not able to trigger that warning right now,
> so this is fine.
Then, either [PATCH v1] or [PATCH v2] would be ok, but v2 may be a bit
better because it's exacltly compatible with that function prototype,
of_property_read_u32(...u32 *).