Re: [PATCH v1 08/10] powerpc/pseries/iommu: Add ddw_property_create() and refactor enable_ddw()
On Mon, 2020-08-31 at 14:34 +1000, Alexey Kardashevskiy wrote:
>
> On 29/08/2020 01:25, Leonardo Bras wrote:
> > On Mon, 2020-08-24 at 15:07 +1000, Alexey Kardashevskiy wrote:
> > > On 18/08/2020 09:40, Leonardo Bras wrote:
> > > > Code used to create a ddw property that was previously scattered in
> > > > enable_ddw() is now gathered in ddw_property_create(), which deals with
> > > > allocation and filling the property, letting it ready for
> > > > of_property_add(), which now occurs in sequence.
> > > >
> > > > This created an opportunity to reorganize the second part of
> > > > enable_ddw():
> > > >
> > > > Without this patch enable_ddw() does, in order:
> > > > kzalloc() property & members, create_ddw(), fill ddwprop inside
> > > > property,
> > > > ddw_list_add(), do tce_setrange_multi_pSeriesLP_walk in all memory,
> > > > of_add_property().
> > > >
> > > > With this patch enable_ddw() does, in order:
> > > > create_ddw(), ddw_property_create(), of_add_property(), ddw_list_add(),
> > > > do tce_setrange_multi_pSeriesLP_walk in all memory.
> > > >
> > > > This change requires of_remove_property() in case anything fails after
> > > > of_add_property(), but we get to do tce_setrange_multi_pSeriesLP_walk
> > > > in all memory, which looks the most expensive operation, only if
> > > > everything else succeeds.
> > > >
> > > > Signed-off-by: Leonardo Bras
> > > > ---
> > > > arch/powerpc/platforms/pseries/iommu.c | 97 +++---
> > > > 1 file changed, 57 insertions(+), 40 deletions(-)
> > > >
> > > > diff --git a/arch/powerpc/platforms/pseries/iommu.c
> > > > b/arch/powerpc/platforms/pseries/iommu.c
> > > > index 4031127c9537..3a1ef02ad9d5 100644
> > > > --- a/arch/powerpc/platforms/pseries/iommu.c
> > > > +++ b/arch/powerpc/platforms/pseries/iommu.c
> > > > @@ -1123,6 +1123,31 @@ static void reset_dma_window(struct pci_dev
> > > > *dev, struct device_node *par_dn)
> > > > ret);
> > > > }
> > > >
> > > > +static int ddw_property_create(struct property **ddw_win, const char
> > > > *propname,
> > >
> > > @propname is always the same, do you really want to pass it every time?
> >
> > I think it reads better, like "create a ddw property with this name".
>
> This reads as "there are at least two ddw properties".
>
> > Also, it makes possible to create ddw properties with other names, in
> > case we decide to create properties with different names depending on
> > the window created.
>
> It is one window at any given moment, why call it different names... I
> get the part that it is not always "direct" anymore but still...
>
It seems the case as one of the options you suggested on patch [09/10]
>> I suspect it breaks kexec (from older kernel to this one) so you
>> either need to check for both DT names or just keep the old one.
>
> > Also, it's probably optimized / inlined at this point.
> > Is it ok doing it like this?
> >
> > > > + u32 liobn, u64 dma_addr, u32 page_shift,
> > > > u32 window_shift)
> > > > +{
> > > > + struct dynamic_dma_window_prop *ddwprop;
> > > > + struct property *win64;
> > > > +
> > > > + *ddw_win = win64 = kzalloc(sizeof(*win64), GFP_KERNEL);
> > > > + if (!win64)
> > > > + return -ENOMEM;
> > > > +
> > > > + win64->name = kstrdup(propname, GFP_KERNEL);
> > >
> > > Not clear why "win64->name = DIRECT64_PROPNAME" would not work here, the
> > > generic OF code does not try kfree() it but it is probably out of scope
> > > here.
> >
> > Yeah, I had that question too.
> > Previous code was like that, and I as trying not to mess too much on
> > how it's done.
> >
> > > > + ddwprop = kzalloc(sizeof(*ddwprop), GFP_KERNEL);
> > > > + win64->value = ddwprop;
> > > > + win64->length = sizeof(*ddwprop);
> > > > + if (!win64->name || !win64->value)
> > > > + return -ENOMEM;
> > >
> > > Up to 2 memory leaks here. I see the cleanup at "out_free_prop:" but
> > > still looks fragile. Instead you could simply return win64 as the only
> > > error possible here is -ENOMEM and returning NULL is equally good.
> >
> > I agree. It's better if this function have it's own cleaning routine.
> > It will be fixed for next version.
> >
> > >
> > > > +
> > > > + ddwprop->liobn = cpu_to_be32(liobn);
> > > > + ddwprop->dma_base = cpu_to_be64(dma_addr);
> > > > + ddwprop->tce_shift = cpu_to_be32(page_shift);
> > > > + ddwprop->window_shift = cpu_to_be32(window_shift);
> > > > +
> > > > + return 0;
> > > > +}
> > > > +
> > > > /*
> > > > * If the PE supports dynamic dma windows, and there is space for a
> > > > table
> > > > * that can map all pages in a linear offset, then setup such a table,
> > > > @@ -1140,12 +1165,11 @@ static bool enable_ddw(struct pci_dev *dev,
> > > > struct device_node *pdn)
> > > > struct ddw_query_response query;
> > > > struct ddw_create_response
Re: [PATCH v1 08/10] powerpc/pseries/iommu: Add ddw_property_create() and refactor enable_ddw()
On 29/08/2020 01:25, Leonardo Bras wrote:
> On Mon, 2020-08-24 at 15:07 +1000, Alexey Kardashevskiy wrote:
>>
>> On 18/08/2020 09:40, Leonardo Bras wrote:
>>> Code used to create a ddw property that was previously scattered in
>>> enable_ddw() is now gathered in ddw_property_create(), which deals with
>>> allocation and filling the property, letting it ready for
>>> of_property_add(), which now occurs in sequence.
>>>
>>> This created an opportunity to reorganize the second part of enable_ddw():
>>>
>>> Without this patch enable_ddw() does, in order:
>>> kzalloc() property & members, create_ddw(), fill ddwprop inside property,
>>> ddw_list_add(), do tce_setrange_multi_pSeriesLP_walk in all memory,
>>> of_add_property().
>>>
>>> With this patch enable_ddw() does, in order:
>>> create_ddw(), ddw_property_create(), of_add_property(), ddw_list_add(),
>>> do tce_setrange_multi_pSeriesLP_walk in all memory.
>>>
>>> This change requires of_remove_property() in case anything fails after
>>> of_add_property(), but we get to do tce_setrange_multi_pSeriesLP_walk
>>> in all memory, which looks the most expensive operation, only if
>>> everything else succeeds.
>>>
>>> Signed-off-by: Leonardo Bras
>>> ---
>>> arch/powerpc/platforms/pseries/iommu.c | 97 +++---
>>> 1 file changed, 57 insertions(+), 40 deletions(-)
>>>
>>> diff --git a/arch/powerpc/platforms/pseries/iommu.c
>>> b/arch/powerpc/platforms/pseries/iommu.c
>>> index 4031127c9537..3a1ef02ad9d5 100644
>>> --- a/arch/powerpc/platforms/pseries/iommu.c
>>> +++ b/arch/powerpc/platforms/pseries/iommu.c
>>> @@ -1123,6 +1123,31 @@ static void reset_dma_window(struct pci_dev *dev,
>>> struct device_node *par_dn)
>>> ret);
>>> }
>>>
>>> +static int ddw_property_create(struct property **ddw_win, const char
>>> *propname,
>>
>> @propname is always the same, do you really want to pass it every time?
>
> I think it reads better, like "create a ddw property with this name".
This reads as "there are at least two ddw properties".
> Also, it makes possible to create ddw properties with other names, in
> case we decide to create properties with different names depending on
> the window created.
It is one window at any given moment, why call it different names... I
get the part that it is not always "direct" anymore but still...
> Also, it's probably optimized / inlined at this point.
> Is it ok doing it like this?
>
>>
>>> + u32 liobn, u64 dma_addr, u32 page_shift, u32
>>> window_shift)
>>> +{
>>> + struct dynamic_dma_window_prop *ddwprop;
>>> + struct property *win64;
>>> +
>>> + *ddw_win = win64 = kzalloc(sizeof(*win64), GFP_KERNEL);
>>> + if (!win64)
>>> + return -ENOMEM;
>>> +
>>> + win64->name = kstrdup(propname, GFP_KERNEL);
>>
>> Not clear why "win64->name = DIRECT64_PROPNAME" would not work here, the
>> generic OF code does not try kfree() it but it is probably out of scope
>> here.
>
> Yeah, I had that question too.
> Previous code was like that, and I as trying not to mess too much on
> how it's done.
>
>>> + ddwprop = kzalloc(sizeof(*ddwprop), GFP_KERNEL);
>>> + win64->value = ddwprop;
>>> + win64->length = sizeof(*ddwprop);
>>> + if (!win64->name || !win64->value)
>>> + return -ENOMEM;
>>
>> Up to 2 memory leaks here. I see the cleanup at "out_free_prop:" but
>> still looks fragile. Instead you could simply return win64 as the only
>> error possible here is -ENOMEM and returning NULL is equally good.
>
> I agree. It's better if this function have it's own cleaning routine.
> It will be fixed for next version.
>
>>
>>
>>> +
>>> + ddwprop->liobn = cpu_to_be32(liobn);
>>> + ddwprop->dma_base = cpu_to_be64(dma_addr);
>>> + ddwprop->tce_shift = cpu_to_be32(page_shift);
>>> + ddwprop->window_shift = cpu_to_be32(window_shift);
>>> +
>>> + return 0;
>>> +}
>>> +
>>> /*
>>> * If the PE supports dynamic dma windows, and there is space for a table
>>> * that can map all pages in a linear offset, then setup such a table,
>>> @@ -1140,12 +1165,11 @@ static bool enable_ddw(struct pci_dev *dev, struct
>>> device_node *pdn)
>>> struct ddw_query_response query;
>>> struct ddw_create_response create;
>>> int page_shift;
>>> - u64 max_addr;
>>> + u64 max_addr, win_addr;
>>> struct device_node *dn;
>>> u32 ddw_avail[DDW_APPLICABLE_SIZE];
>>> struct direct_window *window;
>>> - struct property *win64;
>>> - struct dynamic_dma_window_prop *ddwprop;
>>> + struct property *win64 = NULL;
>>> struct failed_ddw_pdn *fpdn;
>>> bool default_win_removed = false;
>>>
>>> @@ -1244,38 +1268,34 @@ static bool enable_ddw(struct pci_dev *dev, struct
>>> device_node *pdn)
>>> goto out_failed;
>>> }
>>> len = order_base_2(max_addr);
>>> - win64 = kzalloc(sizeof(struct property), GFP_KERNEL);
>>> - if (!win64) {
>>> - dev_info(>dev,
>>> - "couldn't allocate property
Re: [PATCH v1 08/10] powerpc/pseries/iommu: Add ddw_property_create() and refactor enable_ddw()
On Mon, 2020-08-24 at 15:07 +1000, Alexey Kardashevskiy wrote:
>
> On 18/08/2020 09:40, Leonardo Bras wrote:
> > Code used to create a ddw property that was previously scattered in
> > enable_ddw() is now gathered in ddw_property_create(), which deals with
> > allocation and filling the property, letting it ready for
> > of_property_add(), which now occurs in sequence.
> >
> > This created an opportunity to reorganize the second part of enable_ddw():
> >
> > Without this patch enable_ddw() does, in order:
> > kzalloc() property & members, create_ddw(), fill ddwprop inside property,
> > ddw_list_add(), do tce_setrange_multi_pSeriesLP_walk in all memory,
> > of_add_property().
> >
> > With this patch enable_ddw() does, in order:
> > create_ddw(), ddw_property_create(), of_add_property(), ddw_list_add(),
> > do tce_setrange_multi_pSeriesLP_walk in all memory.
> >
> > This change requires of_remove_property() in case anything fails after
> > of_add_property(), but we get to do tce_setrange_multi_pSeriesLP_walk
> > in all memory, which looks the most expensive operation, only if
> > everything else succeeds.
> >
> > Signed-off-by: Leonardo Bras
> > ---
> > arch/powerpc/platforms/pseries/iommu.c | 97 +++---
> > 1 file changed, 57 insertions(+), 40 deletions(-)
> >
> > diff --git a/arch/powerpc/platforms/pseries/iommu.c
> > b/arch/powerpc/platforms/pseries/iommu.c
> > index 4031127c9537..3a1ef02ad9d5 100644
> > --- a/arch/powerpc/platforms/pseries/iommu.c
> > +++ b/arch/powerpc/platforms/pseries/iommu.c
> > @@ -1123,6 +1123,31 @@ static void reset_dma_window(struct pci_dev *dev,
> > struct device_node *par_dn)
> > ret);
> > }
> >
> > +static int ddw_property_create(struct property **ddw_win, const char
> > *propname,
>
> @propname is always the same, do you really want to pass it every time?
I think it reads better, like "create a ddw property with this name".
Also, it makes possible to create ddw properties with other names, in
case we decide to create properties with different names depending on
the window created.
Also, it's probably optimized / inlined at this point.
Is it ok doing it like this?
>
> > + u32 liobn, u64 dma_addr, u32 page_shift, u32
> > window_shift)
> > +{
> > + struct dynamic_dma_window_prop *ddwprop;
> > + struct property *win64;
> > +
> > + *ddw_win = win64 = kzalloc(sizeof(*win64), GFP_KERNEL);
> > + if (!win64)
> > + return -ENOMEM;
> > +
> > + win64->name = kstrdup(propname, GFP_KERNEL);
>
> Not clear why "win64->name = DIRECT64_PROPNAME" would not work here, the
> generic OF code does not try kfree() it but it is probably out of scope
> here.
Yeah, I had that question too.
Previous code was like that, and I as trying not to mess too much on
how it's done.
> > + ddwprop = kzalloc(sizeof(*ddwprop), GFP_KERNEL);
> > + win64->value = ddwprop;
> > + win64->length = sizeof(*ddwprop);
> > + if (!win64->name || !win64->value)
> > + return -ENOMEM;
>
> Up to 2 memory leaks here. I see the cleanup at "out_free_prop:" but
> still looks fragile. Instead you could simply return win64 as the only
> error possible here is -ENOMEM and returning NULL is equally good.
I agree. It's better if this function have it's own cleaning routine.
It will be fixed for next version.
>
>
> > +
> > + ddwprop->liobn = cpu_to_be32(liobn);
> > + ddwprop->dma_base = cpu_to_be64(dma_addr);
> > + ddwprop->tce_shift = cpu_to_be32(page_shift);
> > + ddwprop->window_shift = cpu_to_be32(window_shift);
> > +
> > + return 0;
> > +}
> > +
> > /*
> > * If the PE supports dynamic dma windows, and there is space for a table
> > * that can map all pages in a linear offset, then setup such a table,
> > @@ -1140,12 +1165,11 @@ static bool enable_ddw(struct pci_dev *dev, struct
> > device_node *pdn)
> > struct ddw_query_response query;
> > struct ddw_create_response create;
> > int page_shift;
> > - u64 max_addr;
> > + u64 max_addr, win_addr;
> > struct device_node *dn;
> > u32 ddw_avail[DDW_APPLICABLE_SIZE];
> > struct direct_window *window;
> > - struct property *win64;
> > - struct dynamic_dma_window_prop *ddwprop;
> > + struct property *win64 = NULL;
> > struct failed_ddw_pdn *fpdn;
> > bool default_win_removed = false;
> >
> > @@ -1244,38 +1268,34 @@ static bool enable_ddw(struct pci_dev *dev, struct
> > device_node *pdn)
> > goto out_failed;
> > }
> > len = order_base_2(max_addr);
> > - win64 = kzalloc(sizeof(struct property), GFP_KERNEL);
> > - if (!win64) {
> > - dev_info(>dev,
> > - "couldn't allocate property for 64bit dma window\n");
> > +
> > + ret = create_ddw(dev, ddw_avail, , page_shift, len);
> > + if (ret != 0)
>
> It is usually just "if (ret)"
It was previously like that, and all query_ddw() checks return value
this way. Should I update them all or just this one?
Re: [PATCH v1 08/10] powerpc/pseries/iommu: Add ddw_property_create() and refactor enable_ddw()
On 18/08/2020 09:40, Leonardo Bras wrote:
> Code used to create a ddw property that was previously scattered in
> enable_ddw() is now gathered in ddw_property_create(), which deals with
> allocation and filling the property, letting it ready for
> of_property_add(), which now occurs in sequence.
>
> This created an opportunity to reorganize the second part of enable_ddw():
>
> Without this patch enable_ddw() does, in order:
> kzalloc() property & members, create_ddw(), fill ddwprop inside property,
> ddw_list_add(), do tce_setrange_multi_pSeriesLP_walk in all memory,
> of_add_property().
>
> With this patch enable_ddw() does, in order:
> create_ddw(), ddw_property_create(), of_add_property(), ddw_list_add(),
> do tce_setrange_multi_pSeriesLP_walk in all memory.
>
> This change requires of_remove_property() in case anything fails after
> of_add_property(), but we get to do tce_setrange_multi_pSeriesLP_walk
> in all memory, which looks the most expensive operation, only if
> everything else succeeds.
>
> Signed-off-by: Leonardo Bras
> ---
> arch/powerpc/platforms/pseries/iommu.c | 97 +++---
> 1 file changed, 57 insertions(+), 40 deletions(-)
>
> diff --git a/arch/powerpc/platforms/pseries/iommu.c
> b/arch/powerpc/platforms/pseries/iommu.c
> index 4031127c9537..3a1ef02ad9d5 100644
> --- a/arch/powerpc/platforms/pseries/iommu.c
> +++ b/arch/powerpc/platforms/pseries/iommu.c
> @@ -1123,6 +1123,31 @@ static void reset_dma_window(struct pci_dev *dev,
> struct device_node *par_dn)
>ret);
> }
>
> +static int ddw_property_create(struct property **ddw_win, const char
> *propname,
@propname is always the same, do you really want to pass it every time?
> +u32 liobn, u64 dma_addr, u32 page_shift, u32
> window_shift)
> +{
> + struct dynamic_dma_window_prop *ddwprop;
> + struct property *win64;
> +
> + *ddw_win = win64 = kzalloc(sizeof(*win64), GFP_KERNEL);
> + if (!win64)
> + return -ENOMEM;
> +
> + win64->name = kstrdup(propname, GFP_KERNEL);
Not clear why "win64->name = DIRECT64_PROPNAME" would not work here, the
generic OF code does not try kfree() it but it is probably out of scope
here.
> + ddwprop = kzalloc(sizeof(*ddwprop), GFP_KERNEL);
> + win64->value = ddwprop;
> + win64->length = sizeof(*ddwprop);
> + if (!win64->name || !win64->value)
> + return -ENOMEM;
Up to 2 memory leaks here. I see the cleanup at "out_free_prop:" but
still looks fragile. Instead you could simply return win64 as the only
error possible here is -ENOMEM and returning NULL is equally good.
> +
> + ddwprop->liobn = cpu_to_be32(liobn);
> + ddwprop->dma_base = cpu_to_be64(dma_addr);
> + ddwprop->tce_shift = cpu_to_be32(page_shift);
> + ddwprop->window_shift = cpu_to_be32(window_shift);
> +
> + return 0;
> +}
> +
> /*
> * If the PE supports dynamic dma windows, and there is space for a table
> * that can map all pages in a linear offset, then setup such a table,
> @@ -1140,12 +1165,11 @@ static bool enable_ddw(struct pci_dev *dev, struct
> device_node *pdn)
> struct ddw_query_response query;
> struct ddw_create_response create;
> int page_shift;
> - u64 max_addr;
> + u64 max_addr, win_addr;
> struct device_node *dn;
> u32 ddw_avail[DDW_APPLICABLE_SIZE];
> struct direct_window *window;
> - struct property *win64;
> - struct dynamic_dma_window_prop *ddwprop;
> + struct property *win64 = NULL;
> struct failed_ddw_pdn *fpdn;
> bool default_win_removed = false;
>
> @@ -1244,38 +1268,34 @@ static bool enable_ddw(struct pci_dev *dev, struct
> device_node *pdn)
> goto out_failed;
> }
> len = order_base_2(max_addr);
> - win64 = kzalloc(sizeof(struct property), GFP_KERNEL);
> - if (!win64) {
> - dev_info(>dev,
> - "couldn't allocate property for 64bit dma window\n");
> +
> + ret = create_ddw(dev, ddw_avail, , page_shift, len);
> + if (ret != 0)
It is usually just "if (ret)"
> goto out_failed;
> - }
> - win64->name = kstrdup(DIRECT64_PROPNAME, GFP_KERNEL);
> - win64->value = ddwprop = kmalloc(sizeof(*ddwprop), GFP_KERNEL);
> - win64->length = sizeof(*ddwprop);
> - if (!win64->name || !win64->value) {
> +
> + dev_dbg(>dev, "created tce table LIOBN 0x%x for %pOF\n",
> + create.liobn, dn);
> +
> + win_addr = ((u64)create.addr_hi << 32) | create.addr_lo;
> + ret = ddw_property_create(, DIRECT64_PROPNAME, create.liobn,
> win_addr,
> + page_shift, len);
> + if (ret) {
> dev_info(>dev,
> - "couldn't allocate property name and value\n");
> + "couldn't allocate property, property name, or
> value\n");
> goto out_free_prop;
> }
>
> - ret = create_ddw(dev, ddw_avail,
[PATCH v1 08/10] powerpc/pseries/iommu: Add ddw_property_create() and refactor enable_ddw()
Code used to create a ddw property that was previously scattered in
enable_ddw() is now gathered in ddw_property_create(), which deals with
allocation and filling the property, letting it ready for
of_property_add(), which now occurs in sequence.
This created an opportunity to reorganize the second part of enable_ddw():
Without this patch enable_ddw() does, in order:
kzalloc() property & members, create_ddw(), fill ddwprop inside property,
ddw_list_add(), do tce_setrange_multi_pSeriesLP_walk in all memory,
of_add_property().
With this patch enable_ddw() does, in order:
create_ddw(), ddw_property_create(), of_add_property(), ddw_list_add(),
do tce_setrange_multi_pSeriesLP_walk in all memory.
This change requires of_remove_property() in case anything fails after
of_add_property(), but we get to do tce_setrange_multi_pSeriesLP_walk
in all memory, which looks the most expensive operation, only if
everything else succeeds.
Signed-off-by: Leonardo Bras
---
arch/powerpc/platforms/pseries/iommu.c | 97 +++---
1 file changed, 57 insertions(+), 40 deletions(-)
diff --git a/arch/powerpc/platforms/pseries/iommu.c
b/arch/powerpc/platforms/pseries/iommu.c
index 4031127c9537..3a1ef02ad9d5 100644
--- a/arch/powerpc/platforms/pseries/iommu.c
+++ b/arch/powerpc/platforms/pseries/iommu.c
@@ -1123,6 +1123,31 @@ static void reset_dma_window(struct pci_dev *dev, struct
device_node *par_dn)
ret);
}
+static int ddw_property_create(struct property **ddw_win, const char *propname,
+ u32 liobn, u64 dma_addr, u32 page_shift, u32
window_shift)
+{
+ struct dynamic_dma_window_prop *ddwprop;
+ struct property *win64;
+
+ *ddw_win = win64 = kzalloc(sizeof(*win64), GFP_KERNEL);
+ if (!win64)
+ return -ENOMEM;
+
+ win64->name = kstrdup(propname, GFP_KERNEL);
+ ddwprop = kzalloc(sizeof(*ddwprop), GFP_KERNEL);
+ win64->value = ddwprop;
+ win64->length = sizeof(*ddwprop);
+ if (!win64->name || !win64->value)
+ return -ENOMEM;
+
+ ddwprop->liobn = cpu_to_be32(liobn);
+ ddwprop->dma_base = cpu_to_be64(dma_addr);
+ ddwprop->tce_shift = cpu_to_be32(page_shift);
+ ddwprop->window_shift = cpu_to_be32(window_shift);
+
+ return 0;
+}
+
/*
* If the PE supports dynamic dma windows, and there is space for a table
* that can map all pages in a linear offset, then setup such a table,
@@ -1140,12 +1165,11 @@ static bool enable_ddw(struct pci_dev *dev, struct
device_node *pdn)
struct ddw_query_response query;
struct ddw_create_response create;
int page_shift;
- u64 max_addr;
+ u64 max_addr, win_addr;
struct device_node *dn;
u32 ddw_avail[DDW_APPLICABLE_SIZE];
struct direct_window *window;
- struct property *win64;
- struct dynamic_dma_window_prop *ddwprop;
+ struct property *win64 = NULL;
struct failed_ddw_pdn *fpdn;
bool default_win_removed = false;
@@ -1244,38 +1268,34 @@ static bool enable_ddw(struct pci_dev *dev, struct
device_node *pdn)
goto out_failed;
}
len = order_base_2(max_addr);
- win64 = kzalloc(sizeof(struct property), GFP_KERNEL);
- if (!win64) {
- dev_info(>dev,
- "couldn't allocate property for 64bit dma window\n");
+
+ ret = create_ddw(dev, ddw_avail, , page_shift, len);
+ if (ret != 0)
goto out_failed;
- }
- win64->name = kstrdup(DIRECT64_PROPNAME, GFP_KERNEL);
- win64->value = ddwprop = kmalloc(sizeof(*ddwprop), GFP_KERNEL);
- win64->length = sizeof(*ddwprop);
- if (!win64->name || !win64->value) {
+
+ dev_dbg(>dev, "created tce table LIOBN 0x%x for %pOF\n",
+ create.liobn, dn);
+
+ win_addr = ((u64)create.addr_hi << 32) | create.addr_lo;
+ ret = ddw_property_create(, DIRECT64_PROPNAME, create.liobn,
win_addr,
+ page_shift, len);
+ if (ret) {
dev_info(>dev,
- "couldn't allocate property name and value\n");
+"couldn't allocate property, property name, or
value\n");
goto out_free_prop;
}
- ret = create_ddw(dev, ddw_avail, , page_shift, len);
- if (ret != 0)
+ ret = of_add_property(pdn, win64);
+ if (ret) {
+ dev_err(>dev, "unable to add dma window property for %pOF:
%d",
+ pdn, ret);
goto out_free_prop;
-
- ddwprop->liobn = cpu_to_be32(create.liobn);
- ddwprop->dma_base = cpu_to_be64(((u64)create.addr_hi << 32) |
- create.addr_lo);
- ddwprop->tce_shift = cpu_to_be32(page_shift);
- ddwprop->window_shift = cpu_to_be32(len);
-
- dev_dbg(>dev, "created tce table LIOBN 0x%x for %pOF\n",
- create.liobn, dn);
+
